[haiku-commits] Re: haiku: hrev51162 - in src: tests/kits/shared kits/shared
- From: Dario Casalinuovo <b.vitruvio@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sun, 14 May 2017 13:49:54 +0200
Hi,
On Sun, May 14, 2017 at 11:17 AM, Andrew Lindesay <apl@xxxxxxxxxxxxxx>
wrote:
Hello Dario;
Can you please clarify two of these points for me...
+ return false;
+ }
+ return true;
Multi-line statements should always have brackets (even when the
multi-line is due to indentation) I see a lot of them.
I think this is at Json.cpp:410 at ```case ']':``` --- do you mean that
switch case clauses' statements should be enclosed in braces to give
scope if there is more than one statement?
```
case X:
{
<statement-1>
<statement-N>
break;
}
```
Yes, but this apply also when there's a single statement indented so that
it result in multi-lines, like :
if (something) {
<statement-1-is-verylong-over-80-chars
and-its-indented>;
}
+ if (!NextChar(jsonParseContext, &buffer[0])
+ || !NextChar(jsonParseContext, &buffer[1])
+ || !NextChar(jsonParseContext, &buffer[2])
+ || !NextChar(jsonParseContext, &buffer[3])) {
+ return false;
+ }
Alternate tab indentation level between logical operators.
^^^ Looking in the online style guide at
"someVeryVeryLongConditionThatBarelyFitsOnALine" this seems to have an
additional tab-indent for the following line with the boolean operator.
Can you clarify how that differs from the code above?
In this case it's OK there's no additional parenthesis level following the
codelines strictly. I like to do it this way when there are lots of
statements, definitely I picked the wrong piece of code.
But let's take this :
case 'u':
// unicode escape sequence.
if (!ParseEscapeUnicodeSequence(jsonParseContext,
stringResult)) {
return false;
}
break;
In this snippet, other than the brackets we mentioned you need an
additional indenting level like this :
if (!ParseEscapeUnicodeSequence(jsonParseContext,
stringResult)) {
[...]
There are lots of these in JSon.cpp.
Also a few things came up once I looked at the files :
* JSon.cpp is missing two blank lines after the license text.
* In JSonTextWriter there are various newlines at the end of the files
* At line 401 the if is missing a space before parenthesis.
* Line 217 and 226 there's more than 80 chars. Maybe you can just shorten
the name?
* Does stdio.h is really needed in every file? Just wondering ;)
* JSon.cpp:459 it seems there are two tabs levels, where just one is needed.
* At IncrementLineNumber(), the increment is exectuted after the function
return, is it intentional?
Bye,
Dario
Other related posts:
