string parsing crashes
string parsing crashes
These fixes take care of some crashes I hit when loading psp versions of my player options on Win.
- Attachments
-
[The extension zip has been deactivated and can no longer be displayed.]
Re: string parsing crashes
Looks good.
A very minor comment:
Please try to keep braces on the same line (unless you have a good reason to not do so, which sometimes happens, but you'd rather be ready to justify it).
please use:
please avoid
we don't have any explicit rule for that but I'm strongly thinking about it, so just a heads up for your future patches
A very minor comment:
Please try to keep braces on the same line (unless you have a good reason to not do so, which sometimes happens, but you'd rather be ready to justify it).
please use:
Code: Select all
for (size_t t=0,i=0; t < input.size();t++) {
please avoid
Code: Select all
for (size_t t=0,i=0; t < input.size();t++)
{
Re: string parsing crashes
I'll give you one strong reason why one should put braces on subsequent lines (and this is what we have for coding standards at work): it becomes very easy to see scope.
In one glance, I can tell exactly where each new scope of x starts & ends. Similarly, when you're dealing with giant nests of if {} blocks, this rapidly becomes more useful, esp if you need to copy paste things around within that nested set of braces.
Code: Select all
{
std::string x("foo");
{
std::string x("bar");
{
std::out << x;
if (ConditionCheck() == SomethingElse())
{
string x("lame example, but you get the point");
}
}
}
}
Re: string parsing crashes
We should avoid that as much as possible (I know it is not always the case right now in our code)MootPoint wrote: Similarly, when you're dealing with giant nests of if {} blocks
also, I didn't want you to give me a good "general" reason, but a good reason every time you want to put the bracket on a new line.
Our rule (implicit for now, but explicit soon) is to put opening brackets on the same line. You may disagree because it is more a religious war than anything, but this is our (again, implicit for now) convention.
There are cases where it majorly improves readability (for example, if you init variables in a constructor before the constructor code...) and I am ready to discuss them.
Regarding scope again, I agree with you, but another rule (currently implicit, but soon to become explicit) is to avoid block nesting as much as possible.
Reasons for putting them on the same line:
- reduces the number of lines dramatically, enabling to see more code on the screen in one page
- doesn't screw up IDEs that give you information on the line containing the matching opening bracket when you are looking at a closing one.
In your patch, the "{" changes you made are exactly ones that IMO don't improve readability and just make the function take more space on the screen than it should.
Again, there are good arguments for both things, and I'm not discussing that.
The code conventions in Wagic are not the result of a democratic vote. I ask for people's opinion but I take the decision in the end. I'm evil
Note that my point is not to strongly enforce this rule until it actually becomes one.
My team has the convention of putting brackets at the end of the line, and I admit this majorly influenced my choice. HBL has the same rules as yours, and I keep scr..ing it up, so I'm happy to have rules close to that of my team in my personal project. I know that's a lame excuse.MootPoint wrote:(and this is what we have for coding standards at work)
Re: string parsing crashes
definitely a religious argument, I remember the debates we had at work when finalizing our coding style guidelines
I will argue though that seeing more lines of code isn't necessarily beneficial, it can actually detract from legibility, which I think is the most important goal of maintainable code. Case in point, I find reading this a nightmare:
At a glance, I can't tell where a function starts and where one ends. Here's how I'd format it:
So yeah, I've chewed up a lot of extra vertical space, but chances are if I'm examining the constructor for a bug, I don't really give a crepes that the clone() function is only a few lines away. I now have clear delineation between my functions at a simple glance, and I understand the scope of my functions without having to squint & pay attention to where the braces are landing.
I will argue though that seeing more lines of code isn't necessarily beneficial, it can actually detract from legibility, which I think is the most important goal of maintainable code. Case in point, I find reading this a nightmare:
Code: Select all
ATransformerFOREVER(int id, MTGCardInstance * source, MTGCardInstance * target, string types, string abilities):InstantAbility(id,source,target){
ability = NEW AForeverTransformer(id,source,target,types,abilities);}
int resolve(){
AForeverTransformer * a = ability->clone();
GenericInstantAbility * wrapper = NEW GenericInstantAbility(1,source,(Damageable *)(this->target),a);
wrapper->addToGame();
return 1;}
ATransformerFOREVER * clone() const{
ATransformerFOREVER * a = NEW ATransformerFOREVER(*this);
a->ability = this->ability->clone();
a->isClone = 1;
return a;}
Code: Select all
/*
**
*/
ATransformerFOREVER(int id, MTGCardInstance * source, MTGCardInstance * target, string types, string abilities)
:
InstantAbility(id,source,target)
{
ability = NEW AForeverTransformer(id,source,target,types,abilities);
}
/*
** Also, note here that the construction of GenericInstantAbility() is a little long in the tooth - I might choose here to tab align the
** params, one per line - depends how much horizontal space you deem is acceptable.
*/
int resolve()
{
AForeverTransformer * a = ability->clone();
GenericInstantAbility * wrapper = NEW GenericInstantAbility(1,source,(Damageable *)(this->target),a);
wrapper->addToGame();
return 1;
}
/*
**
*/
ATransformerFOREVER * clone() const
{
ATransformerFOREVER * a = NEW ATransformerFOREVER(*this);
a->ability = this->ability->clone();
a->isClone = 1;
return a;
}
Re: string parsing crashes
I'm on the same side with MootPoint on this one. But as you say, this is your project and only one of many different coding styles around. It would be nice to firm up the coding style guide since there are clearly more people involved now with different coding styles as well as with new devs who are learning how to code. Coding conventions are key to understanding code in an environment like this. It makes the code overall more legible and cohesive.
A couple of other things to think about:
1) spacing between parens and parameters in method signatures and calls
2) variable naming conventions. MootPoint reminded me about this the other day when he noticed I used camel casing for variable naming, when the rest of the code seemed to not use it. (i'll have to fix that later)
3) Is there an order to how you would like the methods, variables in the source files to be declared? ie private, protected, public or public, protected, private
4) indentation of code. currently you've stated 2 spaces, but could we discuss the use of 4 spaces?
there are probably more, and I would like for all of us to have a discussion if possible about these conventions at the appropriate time.
Thanks,
Mike
A couple of other things to think about:
1) spacing between parens and parameters in method signatures and calls
2) variable naming conventions. MootPoint reminded me about this the other day when he noticed I used camel casing for variable naming, when the rest of the code seemed to not use it. (i'll have to fix that later)
3) Is there an order to how you would like the methods, variables in the source files to be declared? ie private, protected, public or public, protected, private
4) indentation of code. currently you've stated 2 spaces, but could we discuss the use of 4 spaces?
there are probably more, and I would like for all of us to have a discussion if possible about these conventions at the appropriate time.
Thanks,
Mike
Re: string parsing crashes
here's another very good example of an aggravation caused by terse coding practices:
Again, if your coding standards force people to do this, you avert the problem:
What if I want to put a breakpoint on any of these error conditions? Uh, I can't. Not without reformatting & recompiling.int GameObserver::receiveEvent(WEvent * e){
if (!e) return 0;
eventsQueue.push(e);
if (eventsQueue.size() > 1) return -1; //resolving events can generate more events
Again, if your coding standards force people to do this, you avert the problem:
int GameObserver::receiveEvent(WEvent * e)
{
if (!e)
return 0;
eventsQueue.push(e);
if (eventsQueue.size() > 1)
return -1; //resolving events can generate more events
Re: string parsing crashes
Ok, let's hijack this topic and discuss coding guidelines, I do want us to have stricter rules after the upcoming release. I'll try to have the other devs have a look at this thread and state their own arguments.
Please answer to the following questions, give examples if you want. I'll update this thread with your choices, you're of course allowed to change your mind as we discuss
1) How should we format opening/closing brackets ?
always on a new line: MootPoint, mnguyen
always on the same line:
On the same line except for functions opening/closing brackets: wololo
2) spacing between parens and parameters in method signatures and calls
int foo(int a, int b) { : wololo
3.a) Should we add brackets after a "if" if there's only one operation inside the if ?
No:
Yes: wololo
3.b) should the unique operation after a "if" be on a separate line?
No:
Yes: wololo, MootPoint
4) Indentation: 2 or 4 spaces ?
2:
4: wololo, mnguyen
5.a) variable naming: camelcase or lowercase+underscore ?
camelcase wololo
lowercase+underscore
5.b) variable naming: how to prefix global variables, class attributes, local variables?
gGlobal, mAttribute, noSpecificRuleForLocalVars : wololo
6) any maximum number of nested blocks depth? (this is of course flexible..it means the coder will be strongly encouraged to refactor when this happens)
No
3: wololo
7) any limit to the size of a line ?
No: wololo
8) What order should we use for variable/methods to be declared in .h files? How about order in .cpp files?
h files
private, protected,public > Attributes then methods > And alphabetical order inside those: wololo
cpp files
No specific order: wololo
9) Anything else? (we can also discuss that later, we don't need all rules at once)
Please answer to the following questions, give examples if you want. I'll update this thread with your choices, you're of course allowed to change your mind as we discuss
1) How should we format opening/closing brackets ?
always on a new line: MootPoint, mnguyen
always on the same line:
On the same line except for functions opening/closing brackets: wololo
2) spacing between parens and parameters in method signatures and calls
int foo(int a, int b) { : wololo
3.a) Should we add brackets after a "if" if there's only one operation inside the if ?
No:
Yes: wololo
3.b) should the unique operation after a "if" be on a separate line?
No:
Yes: wololo, MootPoint
4) Indentation: 2 or 4 spaces ?
2:
4: wololo, mnguyen
5.a) variable naming: camelcase or lowercase+underscore ?
camelcase wololo
lowercase+underscore
5.b) variable naming: how to prefix global variables, class attributes, local variables?
gGlobal, mAttribute, noSpecificRuleForLocalVars : wololo
6) any maximum number of nested blocks depth? (this is of course flexible..it means the coder will be strongly encouraged to refactor when this happens)
No
3: wololo
7) any limit to the size of a line ?
No: wololo
8) What order should we use for variable/methods to be declared in .h files? How about order in .cpp files?
h files
private, protected,public > Attributes then methods > And alphabetical order inside those: wololo
cpp files
No specific order: wololo
9) Anything else? (we can also discuss that later, we don't need all rules at once)
Re: string parsing crashes
1) How should we format opening/closing brackets ? I'm with Wololo, I prefer always on the same line except for opening/closing classes/functions.
2) int foo(int a, int b) { sounds fine.
3.a) No, I don't think it's necessary. I find single-statement if clusters perfectly legible.
3.b) I don't mind moving towards requiring a newline.
4) Four spaces is fine.
5.a) Definitely prefer camelCase.
5.b) gGlobal, mAttribute, noSpecificRuleForLocalVars sounds good.
6) I've a slight preference not to limit, because this'd be on a case-by-case basis anyways. Maybe rather than a limit, we'd make it mandatory to request code reviews when over X blocks? I dunno, it seems like a lot of management overhead would be involved...
7) No hard limit, but be reasonable.
8) I'm fine with ordering based on private/protected/public, and then attributes/methods, but there's no good reason I can see to order beyond that alphabetically. I think it makes much more sense to order by logical groups, having all the file IO in one group, etc, on a class by class basis. Any modern IDE will bring you directly to the function definition from a given call, so I don't see alphabetical ordering as an advantage. On the other hand, logical grouping means that when researching a given function you can immediately see related functions. I'd be all for a requirement that these logical groups be explicitly labeled, though.
9) While we're coming up with rules, I find it immensely helpful to have a single sentence synopsis comment following function member definition. Not as a hard rule, but as a strong suggestion? Really, the more comments the better, in my opinion.
2) int foo(int a, int b) { sounds fine.
3.a) No, I don't think it's necessary. I find single-statement if clusters perfectly legible.
3.b) I don't mind moving towards requiring a newline.
4) Four spaces is fine.
5.a) Definitely prefer camelCase.
5.b) gGlobal, mAttribute, noSpecificRuleForLocalVars sounds good.
6) I've a slight preference not to limit, because this'd be on a case-by-case basis anyways. Maybe rather than a limit, we'd make it mandatory to request code reviews when over X blocks? I dunno, it seems like a lot of management overhead would be involved...
7) No hard limit, but be reasonable.
8) I'm fine with ordering based on private/protected/public, and then attributes/methods, but there's no good reason I can see to order beyond that alphabetically. I think it makes much more sense to order by logical groups, having all the file IO in one group, etc, on a class by class basis. Any modern IDE will bring you directly to the function definition from a given call, so I don't see alphabetical ordering as an advantage. On the other hand, logical grouping means that when researching a given function you can immediately see related functions. I'd be all for a requirement that these logical groups be explicitly labeled, though.
9) While we're coming up with rules, I find it immensely helpful to have a single sentence synopsis comment following function member definition. Not as a hard rule, but as a strong suggestion? Really, the more comments the better, in my opinion.
Re: string parsing crashes
1) How should we format opening/closing brackets ?
=> always on a new line except when I'm not in a mood for it (see after) !!!
2) spacing between parens and parameters in method signatures and calls
=> int foo(int a, int b) { looks good
3.a) Should we add brackets after a "if" if there's only one operation inside the if ?
=> Yes
3.b) should the unique operation after a "if" be on a separate line?
=> Yes
4) Indentation: 2 or 4 spaces ?
=> 2
5.a) variable naming: camelcase or lowercase+underscore ?
=> camelcase
5.b) variable naming: how to prefix global variables, class attributes, local variables?
=> gGlobal, mAttribute, noSpecificRuleForLocalVars
6) any maximum number of nested blocks depth? (this is of course flexible..it means the coder will be strongly encouraged to refactor when this happens)
=> No : if it's flexible, then it's not a rule, we need to find something else, for example, limit the number of line of a method and limit the number of characters per line
7) any limit to the size of a line ?
=> Yes: my company used to force developers to rewrite code if code lines where longer than 80 characters. It's a bit **** currently, but I would say, more than 100 characters should be avoided
8) What order should we use for variable/methods to be declared in .h files? How about order in .cpp files?
=> private, protected,public > Attributes then methods only for .h. No alphabetic order, everybody uses smart editors anyway
9) Anything else?
=> We all agree on no exception ?
=> Any restriction on the usage of template and macro ? They can make the one writing them very proud but they can also make the ones using or modifying them very unhappy depending of their complexity ...
=> return point in method. My company forces every method to have a single return point. I think it's also a practice applied in the Linux Kernel development. It increases a lot the readability once it's correctly applied correctly and everybody is used to it. It can also be used to remove nested blocks.
For example, without it, you get:
With it you get:
=> always on a new line except when I'm not in a mood for it (see after) !!!
2) spacing between parens and parameters in method signatures and calls
=> int foo(int a, int b) { looks good
3.a) Should we add brackets after a "if" if there's only one operation inside the if ?
=> Yes
3.b) should the unique operation after a "if" be on a separate line?
=> Yes
4) Indentation: 2 or 4 spaces ?
=> 2
5.a) variable naming: camelcase or lowercase+underscore ?
=> camelcase
5.b) variable naming: how to prefix global variables, class attributes, local variables?
=> gGlobal, mAttribute, noSpecificRuleForLocalVars
6) any maximum number of nested blocks depth? (this is of course flexible..it means the coder will be strongly encouraged to refactor when this happens)
=> No : if it's flexible, then it's not a rule, we need to find something else, for example, limit the number of line of a method and limit the number of characters per line
7) any limit to the size of a line ?
=> Yes: my company used to force developers to rewrite code if code lines where longer than 80 characters. It's a bit **** currently, but I would say, more than 100 characters should be avoided
8) What order should we use for variable/methods to be declared in .h files? How about order in .cpp files?
=> private, protected,public > Attributes then methods only for .h. No alphabetic order, everybody uses smart editors anyway
9) Anything else?
=> We all agree on no exception ?
=> Any restriction on the usage of template and macro ? They can make the one writing them very proud but they can also make the ones using or modifying them very unhappy depending of their complexity ...
=> return point in method. My company forces every method to have a single return point. I think it's also a practice applied in the Linux Kernel development. It increases a lot the readability once it's correctly applied correctly and everybody is used to it. It can also be used to remove nested blocks.
For example, without it, you get:
Code: Select all
int foo()
{
if(somethingHappened)
{
return somethingSmart;
}
else
{
if(reallySure)
{
return somethingDumb;
}
else
{
return somethingNotThatDumb;
}
}
return whatever;
}
Code: Select all
int foo()
{
int something;
do { // I like having those on the same line, but not the others !!
if(somethingHappened)
{
something = somethingSmart;
break;
}
if(reallySure)
{
something = somethingDumb;
break;
}
else
{
something = somethingNotThatDumb;
break;
}
something = whatever;
} while(0);
return something;
}