You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@corinthia.apache.org by Gabriela Gibson <ga...@gmail.com> on 2015/05/04 00:52:53 UTC
Operations.c: DFGet() called from consumers/dfconvert/main.c
Right now, consumers/dfconvert/main.c is doing a lot of work but only
checks it has a file to put it in at the very end of it's effort, in
Operations.c. If the file already exists, it bails out with an a error.
I would propose that we do this instead of that, in Operations.c: 355, ie,
we move the check right up to to point at which we can immediately tell if
the work done is in vain:
int DFGetFile(const char *concreteFilename,
const char *abstractFilename,
DFError **error)
{
int success = 0;
if (DFFileExists(abstractFilename)) {
DFErrorFormat(error,
"%s: File already exists",
abstractFilename);
return success;
}
...
I also would like to rename int r = 0; to the (a little more obvious) int
success = 0;
What do you think?
G
--
Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
Posted by jan i <ja...@apache.org>.
On Monday, May 4, 2015, Gabriela Gibson <ga...@gmail.com> wrote:
> On Mon, May 4, 2015 at 2:09 PM, Franz de Copenhague <
> franzdecopenhague@outlook.com <javascript:;>> wrote:
>
> > ----------------------------------------
> > > Date: Mon, 4 May 2015 07:59:31 +0200
> > > Subject: Re: Operations.c: DFGet() called from
> consumers/dfconvert/main.c
> > > From: jani@apache.org <javascript:;>
> > > To: dev@corinthia.incubator.apache.org <javascript:;>
> > >
> > > On Monday, May 4, 2015, Gabriela Gibson <gabriela.gibson@gmail.com
> <javascript:;>>
> > wrote:
> > >
> > >>
> > >> I also would like to rename int r = 0; to the (a little more obvious)
> > int
> > >> success = 0;
> > >>
> > >> What do you think?
> > >
> > > sounds logical to me.
> >
> > To keep the semantics you can do r = SUCCESS instead of r = 0; and define
> > SUCCESS somewhere
> >
> > That would be a global decision --- and I think quite a good one, since
> this kind of pattern will come up a number of times in the code and it's
> nice to have uniformity.
>
> I'm not sure in which header file this would live in, so that it can be
> found everywhere.
>
> What does everyone think about this?
platform.h comes into mind, since theoretically a new platform might
redefine it.
just a suggestion, no strong opinion.
rgds
jan i
>
> G
>
>
>
>
>
>
> --
> Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
>
--
Sent from My iPad, sorry for any misspellings.
Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
Posted by jan i <ja...@apache.org>.
On Monday, May 4, 2015, Peter Kelly <pm...@apache.org> wrote:
> > On 4 May 2015, at 8:39 pm, Gabriela Gibson <gabriela.gibson@gmail.com
> <javascript:;>> wrote:
> >
> > On Mon, May 4, 2015 at 2:09 PM, Franz de Copenhague <
> > franzdecopenhague@outlook.com <javascript:;>> wrote:
> >
> >> ----------------------------------------
> >>> Date: Mon, 4 May 2015 07:59:31 +0200
> >>> Subject: Re: Operations.c: DFGet() called from
> consumers/dfconvert/main.c
> >>> From: jani@apache.org <javascript:;>
> >>> To: dev@corinthia.incubator.apache.org <javascript:;>
> >>>
> >>> On Monday, May 4, 2015, Gabriela Gibson <gabriela.gibson@gmail.com
> <javascript:;>>
> >> wrote:
> >>>
> >>>>
> >>>> I also would like to rename int r = 0; to the (a little more obvious)
> >> int
> >>>> success = 0;
> >>>>
> >>>> What do you think?
> >>>
> >>> sounds logical to me.
> >>
> >> To keep the semantics you can do r = SUCCESS instead of r = 0; and
> define
> >> SUCCESS somewhere
> >>
> >> That would be a global decision --- and I think quite a good one, since
> > this kind of pattern will come up a number of times in the code and it's
> > nice to have uniformity.
> >
> > I'm not sure in which header file this would live in, so that it can be
> > found everywhere.
> >
> > What does everyone think about this?
>
> I think naming the variable to ‘success’ would be the better of the two.
> This is the same semantics, just more obvious to the reader. I’ve used the
> name ‘ok’ in a number of places as it’s shorter to type, but either name is
> clear.
>
> A SUCCESS constant introduces the risk that someone might write:
>
> if (r == SUCCESS)
>
> which is incorrect, as in C any non-zero value is considered to be true.
> The following is clearer:
>
> if (success)
>
> or
>
> if (ok)
>
> See the WordGet function in DocFormats/filters/ooxml/src/word/Word.c for
> an example.
seems we replied in parallel, I prefer peters solution.
rgds
jan i
>
> —
> Dr Peter M. Kelly
> pmkelly@apache.org <javascript:;>
>
> PGP key: http://www.kellypmk.net/pgp-key <http://www.kellypmk.net/pgp-key>
> (fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966)
>
>
--
Sent from My iPad, sorry for any misspellings.
Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
Posted by jan i <ja...@apache.org>.
On 4 May 2015 at 17:02, Gabriela Gibson <ga...@gmail.com> wrote:
> In that case, I'll chance 'success' to 'ok', because that preserves
> uniformity (and it's shorter too :)
>
> Whilst it seems like a small thing, I personally find consistency makes
> skim reading much easier and quicker, the less one has to read(and parse),
> the more brain space is left for thinking.
>
you are quite right here....but you will find that programmers are "lazy"
and do not use the names they should use.
Thanks for correcting this. Even though I am less active online at the
moment (I am caught up in a battle in some higher levels, which actually
might end up changing a lot for me personally), I still follow what is
happening actively, and it pleased me a lot to see that you started digging
into ODF:
Keep up the good work !!
rgds
jan I.
>
> G
>
> On Mon, May 4, 2015 at 3:13 PM, Peter Kelly <pm...@apache.org> wrote:
>
> > > On 4 May 2015, at 8:39 pm, Gabriela Gibson <ga...@gmail.com>
> > wrote:
> > >
> > > On Mon, May 4, 2015 at 2:09 PM, Franz de Copenhague <
> > > franzdecopenhague@outlook.com> wrote:
> > >
> > >> ----------------------------------------
> > >>> Date: Mon, 4 May 2015 07:59:31 +0200
> > >>> Subject: Re: Operations.c: DFGet() called from
> > consumers/dfconvert/main.c
> > >>> From: jani@apache.org
> > >>> To: dev@corinthia.incubator.apache.org
> > >>>
> > >>> On Monday, May 4, 2015, Gabriela Gibson <ga...@gmail.com>
> > >> wrote:
> > >>>
> > >>>>
> > >>>> I also would like to rename int r = 0; to the (a little more
> obvious)
> > >> int
> > >>>> success = 0;
> > >>>>
> > >>>> What do you think?
> > >>>
> > >>> sounds logical to me.
> > >>
> > >> To keep the semantics you can do r = SUCCESS instead of r = 0; and
> > define
> > >> SUCCESS somewhere
> > >>
> > >> That would be a global decision --- and I think quite a good one,
> since
> > > this kind of pattern will come up a number of times in the code and
> it's
> > > nice to have uniformity.
> > >
> > > I'm not sure in which header file this would live in, so that it can be
> > > found everywhere.
> > >
> > > What does everyone think about this?
> >
> > I think naming the variable to ‘success’ would be the better of the two.
> > This is the same semantics, just more obvious to the reader. I’ve used
> the
> > name ‘ok’ in a number of places as it’s shorter to type, but either name
> is
> > clear.
> >
> > A SUCCESS constant introduces the risk that someone might write:
> >
> > if (r == SUCCESS)
> >
> > which is incorrect, as in C any non-zero value is considered to be true.
> > The following is clearer:
> >
> > if (success)
> >
> > or
> >
> > if (ok)
> >
> > See the WordGet function in DocFormats/filters/ooxml/src/word/Word.c for
> > an example.
> >
> > —
> > Dr Peter M. Kelly
> > pmkelly@apache.org
> >
> > PGP key: http://www.kellypmk.net/pgp-key <
> http://www.kellypmk.net/pgp-key>
> > (fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966)
> >
> >
>
>
> --
> Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
>
Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
Posted by Gabriela Gibson <ga...@gmail.com>.
In that case, I'll chance 'success' to 'ok', because that preserves
uniformity (and it's shorter too :)
Whilst it seems like a small thing, I personally find consistency makes
skim reading much easier and quicker, the less one has to read(and parse),
the more brain space is left for thinking.
G
On Mon, May 4, 2015 at 3:13 PM, Peter Kelly <pm...@apache.org> wrote:
> > On 4 May 2015, at 8:39 pm, Gabriela Gibson <ga...@gmail.com>
> wrote:
> >
> > On Mon, May 4, 2015 at 2:09 PM, Franz de Copenhague <
> > franzdecopenhague@outlook.com> wrote:
> >
> >> ----------------------------------------
> >>> Date: Mon, 4 May 2015 07:59:31 +0200
> >>> Subject: Re: Operations.c: DFGet() called from
> consumers/dfconvert/main.c
> >>> From: jani@apache.org
> >>> To: dev@corinthia.incubator.apache.org
> >>>
> >>> On Monday, May 4, 2015, Gabriela Gibson <ga...@gmail.com>
> >> wrote:
> >>>
> >>>>
> >>>> I also would like to rename int r = 0; to the (a little more obvious)
> >> int
> >>>> success = 0;
> >>>>
> >>>> What do you think?
> >>>
> >>> sounds logical to me.
> >>
> >> To keep the semantics you can do r = SUCCESS instead of r = 0; and
> define
> >> SUCCESS somewhere
> >>
> >> That would be a global decision --- and I think quite a good one, since
> > this kind of pattern will come up a number of times in the code and it's
> > nice to have uniformity.
> >
> > I'm not sure in which header file this would live in, so that it can be
> > found everywhere.
> >
> > What does everyone think about this?
>
> I think naming the variable to ‘success’ would be the better of the two.
> This is the same semantics, just more obvious to the reader. I’ve used the
> name ‘ok’ in a number of places as it’s shorter to type, but either name is
> clear.
>
> A SUCCESS constant introduces the risk that someone might write:
>
> if (r == SUCCESS)
>
> which is incorrect, as in C any non-zero value is considered to be true.
> The following is clearer:
>
> if (success)
>
> or
>
> if (ok)
>
> See the WordGet function in DocFormats/filters/ooxml/src/word/Word.c for
> an example.
>
> —
> Dr Peter M. Kelly
> pmkelly@apache.org
>
> PGP key: http://www.kellypmk.net/pgp-key <http://www.kellypmk.net/pgp-key>
> (fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966)
>
>
--
Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
Posted by Peter Kelly <pm...@apache.org>.
> On 4 May 2015, at 8:39 pm, Gabriela Gibson <ga...@gmail.com> wrote:
>
> On Mon, May 4, 2015 at 2:09 PM, Franz de Copenhague <
> franzdecopenhague@outlook.com> wrote:
>
>> ----------------------------------------
>>> Date: Mon, 4 May 2015 07:59:31 +0200
>>> Subject: Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
>>> From: jani@apache.org
>>> To: dev@corinthia.incubator.apache.org
>>>
>>> On Monday, May 4, 2015, Gabriela Gibson <ga...@gmail.com>
>> wrote:
>>>
>>>>
>>>> I also would like to rename int r = 0; to the (a little more obvious)
>> int
>>>> success = 0;
>>>>
>>>> What do you think?
>>>
>>> sounds logical to me.
>>
>> To keep the semantics you can do r = SUCCESS instead of r = 0; and define
>> SUCCESS somewhere
>>
>> That would be a global decision --- and I think quite a good one, since
> this kind of pattern will come up a number of times in the code and it's
> nice to have uniformity.
>
> I'm not sure in which header file this would live in, so that it can be
> found everywhere.
>
> What does everyone think about this?
I think naming the variable to ‘success’ would be the better of the two. This is the same semantics, just more obvious to the reader. I’ve used the name ‘ok’ in a number of places as it’s shorter to type, but either name is clear.
A SUCCESS constant introduces the risk that someone might write:
if (r == SUCCESS)
which is incorrect, as in C any non-zero value is considered to be true. The following is clearer:
if (success)
or
if (ok)
See the WordGet function in DocFormats/filters/ooxml/src/word/Word.c for an example.
—
Dr Peter M. Kelly
pmkelly@apache.org
PGP key: http://www.kellypmk.net/pgp-key <http://www.kellypmk.net/pgp-key>
(fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966)
Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
Posted by Gabriela Gibson <ga...@gmail.com>.
On Mon, May 4, 2015 at 2:09 PM, Franz de Copenhague <
franzdecopenhague@outlook.com> wrote:
> ----------------------------------------
> > Date: Mon, 4 May 2015 07:59:31 +0200
> > Subject: Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
> > From: jani@apache.org
> > To: dev@corinthia.incubator.apache.org
> >
> > On Monday, May 4, 2015, Gabriela Gibson <ga...@gmail.com>
> wrote:
> >
> >>
> >> I also would like to rename int r = 0; to the (a little more obvious)
> int
> >> success = 0;
> >>
> >> What do you think?
> >
> > sounds logical to me.
>
> To keep the semantics you can do r = SUCCESS instead of r = 0; and define
> SUCCESS somewhere
>
> That would be a global decision --- and I think quite a good one, since
this kind of pattern will come up a number of times in the code and it's
nice to have uniformity.
I'm not sure in which header file this would live in, so that it can be
found everywhere.
What does everyone think about this?
G
--
Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
RE: Operations.c: DFGet() called from consumers/dfconvert/main.c
Posted by Franz de Copenhague <fr...@outlook.com>.
----------------------------------------
> Date: Mon, 4 May 2015 07:59:31 +0200
> Subject: Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
> From: jani@apache.org
> To: dev@corinthia.incubator.apache.org
>
> On Monday, May 4, 2015, Gabriela Gibson <ga...@gmail.com> wrote:
>
>> Right now, consumers/dfconvert/main.c is doing a lot of work but only
>> checks it has a file to put it in at the very end of it's effort, in
>> Operations.c. If the file already exists, it bails out with an a error.
>>
>> I would propose that we do this instead of that, in Operations.c: 355, ie,
>> we move the check right up to to point at which we can immediately tell if
>> the work done is in vain:
>>
>> int DFGetFile(const char *concreteFilename,
>> const char *abstractFilename,
>> DFError **error)
>> {
>> int success = 0;
>>
>> if (DFFileExists(abstractFilename)) {
>> DFErrorFormat(error,
>> "%s: File already exists",
>> abstractFilename);
>> return success;
>> }
>>
>> ...
>>
>> I also would like to rename int r = 0; to the (a little more obvious) int
>> success = 0;
>>
>> What do you think?
>
> sounds logical to me.
To keep the semantics you can do r = SUCCESS instead of r = 0; and define SUCCESS somewhere
franz
Re: Operations.c: DFGet() called from consumers/dfconvert/main.c
Posted by jan i <ja...@apache.org>.
On Monday, May 4, 2015, Gabriela Gibson <ga...@gmail.com> wrote:
> Right now, consumers/dfconvert/main.c is doing a lot of work but only
> checks it has a file to put it in at the very end of it's effort, in
> Operations.c. If the file already exists, it bails out with an a error.
>
> I would propose that we do this instead of that, in Operations.c: 355, ie,
> we move the check right up to to point at which we can immediately tell if
> the work done is in vain:
>
> int DFGetFile(const char *concreteFilename,
> const char *abstractFilename,
> DFError **error)
> {
> int success = 0;
>
> if (DFFileExists(abstractFilename)) {
> DFErrorFormat(error,
> "%s: File already exists",
> abstractFilename);
> return success;
> }
>
> ...
>
> I also would like to rename int r = 0; to the (a little more obvious) int
> success = 0;
>
> What do you think?
sounds logical to me.
rgds
jan i
>
> G
>
> --
> Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
>
--
Sent from My iPad, sorry for any misspellings.