[haiku-commits] Re: haiku: hrev43975 - src/apps/deskbar
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Thu, 12 Apr 2012 12:23:44 +0200
On 2012-04-11 at 21:10:27 [+0200], John Scipione <jscipione@xxxxxxxxx> wrote:
> On Wed, Apr 11, 2012 at 10:28 AM, Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
> wrote:
> > On 08.04.2012 03:33, jscipione@xxxxxxxxx wrote:
> >>
> >> 8ff49ae: Use != B_OK, not < B_OK to indicate an error.
> >> Apparently error codes are allowed to be positive.
> >
> > No, error codes are negative on Haiku; for example, the ssize_t type makes
> > this mandatory. When the type is status_t, however, you should not assume
> > anything about the sign of an error code; for all we know, a status_t
> > should
> > never be positive, so an error check like this may hide a potential
> > programming error.
>
> In this case I am checking the return value of BMessage->FindXXXX()
> which I know returns a status_t that is either an error code
> (negative) on error or B_OK (0) on success. So I guess if
> BMessage->FindXXXX() returned a positive number that would be very bad
> indeed. So you are right, I should check != in case something really
> screwy happens.
>
> Thanks for the explanation. I've seen < B_OK and != B_OK in various
> places in the code and didn't know when to use which. For a general
> rule != B_OK should be used to check between success/error unless the
> function can return a positive number on success.
Usually those functions return a value >= 0 on success, so the consequent
error check would be < 0, not < B_OK.
> I'd love to see that
> added to the style guide.
Certainly doesn't harm.
> > Anyway, the != B_OK of the second line of the if-clauses should be
> > indented
> > one tab further; at least our current practice is to only indent the
> > logical
> > and/or operators on the same level as the if.
>
> If you want me to follow this rule it must be added to the style
> guide. I tend to follow the conventions used by surrounding code
> unless it is contradicted by the style guide.
>
> The style guide says: indent one tab per expression level, that's is
> what I did. If there is an exception for non-logical operators that
> needs to be stated more clearly.
>
> Also, I am guessing these lines should be indented too:
>
> if (storedSettings.FindInt32("recentFoldersCount",
> &settings.recentFoldersCount) != B_OK) {
> settings.recentFoldersCount = 10;
> }
>
> Since they do not start with a logical operator.
I believe the rule is not to additionally indent when the top level logical
expression (usually a && or ||) is wrapped at their operators, but indent an
additional level when wrapping an inner expression (or function call in
case). More indentation levels are possible, if the expression is long and
you have to wrap it at different levels. Generally good practice is to make
things well readable. I.e. in case of an if expression, wrap at opportune
places (instead of greedily using all line space) and indent to match the
structure of the expression.
> I don't mean to yell at you, but, I have been down this path before. I
> want to follow the style rules but I can't. I can't rely on the style
> of the surrounding code always being correct, and if there isn't a
> rule about something in the style guide I can't follow it. So, if
> there are rules in your head that are not in the style guide you can't
> expect me to follow them since I have no way of knowing what they are.
The preferred style has not been fixed for the last 10 years. It has evolved
over time and the style guidelines weren't nearly complete to begin with. We
can just add a rule to the document (or change one) when we realize it that
is necessary. As usual someone has to do it, though. :-)
CU, Ingo
Other related posts:
