string parsing crashes

All code submission.
wololo
Site Admin
Posts: 3727
Joined: Wed Oct 15, 2008 12:42 am
Location: Japan

Re: string parsing crashes

Post by wololo » Tue Nov 16, 2010 11:57 am

ok, so, kind of "back to the topic" while staying in line with Zethfox's request, how about the following (based on webKit's rule above)?

Variable naming: on top of the prefixing rule, For globals, constants, and class attributes, use full words, except in the rare case where an abbreviation would be more canonical and easier to understand. For local variables, use your best judgment and a balance between readability and efficiency.

Does that sound ok?
I personally have a tendency to use very short variable names. "s" for a string, etc...
I understand MootPoint and Zethfox's point regarding tq, tc, etc... but I think it should be up to each one of us. I still strongly think that "tc" for a TargetChooser instance, given it's very wide usage, should be accepted. So I think the best compromise I can come up with for now is "use your best judgment", because I don't like to type long variable names (which to me is what cripples Java code)

I think enforcing a "full name" rule on class variables, constant, globals will improve readability, while not setting any specific rules (except Camelcase)on local variables still gives everyone some flexibility without sacrificing too much of readability.

Second point: constants.

Should we use kPrefix, or UPPERCASE ?

I'm in favor of UPPERCASE because we have more constants using that for now, but I won't fight for it if people prefer kPrefix, so vote now :)

Once we clear these two points I'll update the official rules page :)

Zethfox
Posts: 3022
Joined: Thu Jun 10, 2010 11:28 pm

Re: string parsing crashes

Post by Zethfox » Tue Nov 16, 2010 1:50 pm

jean i think you miss understood, about

ATK

in most games ive played in here the states, card games, roleplaying games, ect
the acronym "ATK" is often used for "ATTACK" which makes sense,

in the case of wagic however, we are defining an attacker as "ATK" in one section,
as "DEFENSER" in another, (which mind you a defenser in NOT the blocker) it is the creature being blocked AKA the "attacker"
as "attacker" in another....
do you see where this starts to get overly complicated?
they could have all just been written "Attacker"...
with extra consideration for "ATK" since this to me means "Attack" and close to "attackpower" then anything else.

in no way am i saying that the code is bad, wrong ect...but the use of shorthand and abbrivations, and inconsistencies in naming scheme make ALOT of the code hard to pick up even for a professional coder. i think mike can say something about this....mike mind telling us how the target chooser changling issue panned out for you? how hard was it finding out how the target system worked, and how hard would it have been if i didnt walk you through some of the functions to explain (what i gruesomely disected) what they did?

i will say, abbriavtion are fine this is candy, sure...however...mindlessly doing it just to do it, makes no sense and in no way is a 2 letter abbriavtion good, i dont see how you can make sense of this. even if it only survives for 1 line, thats no excuse to use "ActAtKr" to mean "activeAttacker" much less using something like "AK" for it...im a noob and this is how i feel, at what point do professional coders just stop giving an "f"?

also i see that its clear that you(the general public) think im saying make it easier for a noob to pick up...HA hardly....
hey mike, did you ever finish that changeling project? how about multitarget?

EXACTLY....even a professional coder has a hard time understanding some of the more cryptically written functions.
i bet if it didnt use so much of this, what i like to call "comfy coding" and infact what this book im reading calls "comfy coding"
( :P learning stuff already)
it would be ALOT easier for even people like Moot and Mike to do their massive projects without having to spend 20-30 mins trying to find out what an acronym stands for by stepping through the processes.


also about

m

k

yes....i KNOW what theyre for, this is great, however....how is it that it makes sense to give only SOME constants a "K" and SOME member functions a "m"..ect ect....do you see my point? if someone, in this case Mike, wants to start marking all constants with a "K" then he should take it opon himself to insure that the rest of the code is consistent with his new "K" deal....understand?....otherwise he needs to remove his "K"...so it really is 2 roads, remove them or update the source so all constants match up with the new system.

its like i keep reading from you guys, just because BEFORE it was ok to frankstain the code with all sorts of styles and formats, does this mean we should keep doing so? such as adding a new way to mark constants using "K" infront without updating the other 100+ constants already in wagic?

mnguyen
Posts: 229
Joined: Thu Apr 29, 2010 4:13 pm

Re: string parsing crashes

Post by mnguyen » Tue Nov 16, 2010 4:28 pm

again. Since I was the first to introduce this notation I am more than happy to remove it if the majority agrees on it. This is why guidelines are paramount to prevent these types of incongruities. As I have always stated consistency is key.

As far as projects go, they are mainly given to time and combined effort. I've shied away from doing that at the moment as there are huge consequences if I start unrelated to Wagic. So for the time being I'm gathering rough ideas on how to approach the problem and working on things that are a little less destabilizing ( ie menus ). I don't think there is a deadline for any of these projects as we all have our own deadlines in real life. As you will find out, over time some of these things become easier to code over time as they are tossed around and rethought some. Not everything needs to be coded at this moment. Here's an analogy that will help illustrate I hope:
You can always add tires to a car to increase it's forward or backward speed. As you add more tires (features) it becomes harder and harder to turn the car. However, there needs to be a time that you have to stop adding tires and redesign the car before it gets too difficult to steer. Perhaps replace 3-4 sets of tires with one big one. Or replace it's transmission with a higher performing one. The point is, these things take time that is in short supply but high demand. The variable naming convention is not the biggest hurdle for this to take place.

--Mike

wololo
Site Admin
Posts: 3727
Joined: Wed Oct 15, 2008 12:42 am
Location: Japan

Re: string parsing crashes

Post by wololo » Tue Nov 16, 2010 10:14 pm

I think everything that needed to be said has been said, and the latest posts are just repeating previously mentioned arguments.

Edit: regarding Defenser, as I said already, here's the reasoning behind it
CreatureA->defenser is a pointer to another creature.
it is set to null when CreatureA is not blocking, and set to the creature being blocked when CreatureA is blocking.
Therefore:
if (CreatureA->defenser) is equivalent to asking: "is CreatureA blocking anybody?"
and: MTGCardInstance * attackerBlocked = CreatureA->defenser is equivalent to : "get me the creature that CreatureA is blocking"
I understand this is not easy to get, probably a bad design, but still makes some sense. Would probably make more sense to you if it was named "creatureIAmCurrentlyBlocking"


In a desperate attempt to re-focus this thread, everybody please answer to these two questions:

What do you think of this additional rule?
Variable naming: on top of the prefixing rule, For globals, constants, and class attributes, use full words, except in the rare case where an abbreviation would be more canonical and easier to understand. For local variables, use your best judgment and a balance between readability and efficiency.

Second point: constants.
Should we use kPrefix, or UPPERCASE ?


If I don't have any answer by the end of the week, I'll add the rule for naming, and we'll use uppercase for constants.
Zethfox wrote:yes....i KNOW what theyre for, this is great, however....how is it that it makes sense to give only SOME constants a "K" and SOME member functions a "m"..ect ect....
It doesn't, this is why we created this thread, in order to decide for some rules, so please everybody answer the two questions abov and let's be done with this thread.

mnguyen
Posts: 229
Joined: Thu Apr 29, 2010 4:13 pm

Re: string parsing crashes

Post by mnguyen » Tue Nov 16, 2010 10:43 pm

I like the naming rule and constants should be in caps with underscores to separate words.

Jean
Posts: 32
Joined: Tue Nov 18, 2008 5:12 am

Re: string parsing crashes

Post by Jean » Tue Nov 16, 2010 11:55 pm

wololo wrote: Variable naming: on top of the prefixing rule, For globals, constants, and class attributes, use full words, except in the rare case where an abbreviation would be more canonical and easier to understand. For local variables, use your best judgment and a balance between readability and efficiency.
I like this rule as is. We can add an additional rule which would read "identifiers under four letters must have a lifespan under five lines". Or, "up to 10 lines of lifespan, identifiers should have at least as many characters as their lifespan has lines" (which sounds hard to actually check, but pretty nice - let me try this regardless in my own code :) ). If you think this is too tiresome, just leave this rule as is.
wololo wrote: Second point: constants.
Should we use kPrefix, or UPPERCASE ?
I like the UPPERCASE system (I like making constants visible), but since we have mAttributes and gGlobals I'd also find kConstants pretty consistent. So no real vote for either on my part.

Also I'd like to state what is meant by "constant". I mean, obviously a static const int is, as well as a #define for a pure expression. How about enums? How about integral template parameters? How about immutable values?

mnguyen
Posts: 229
Joined: Thu Apr 29, 2010 4:13 pm

Re: string parsing crashes

Post by mnguyen » Wed Nov 17, 2010 12:06 am

Jean wrote:
wololo wrote: Variable naming: on top of the prefixing rule, For globals, constants, and class attributes, use full words, except in the rare case where an abbreviation would be more canonical and easier to understand. For local variables, use your best judgment and a balance between readability and efficiency.
I like this rule as is. We can add an additional rule which would read "identifiers under four letters must have a lifespan under five lines". Or, "up to 10 lines of lifespan, identifiers should have at least as many characters as their lifespan has lines" (which sounds hard to actually check, but pretty nice - let me try this regardless in my own code :) ). If you think this is too tiresome, just leave this rule as is.
wololo wrote: Second point: constants.
Should we use kPrefix, or UPPERCASE ?
I like the UPPERCASE system (I like making constants visible), but since we have mAttributes and gGlobals I'd also find kConstants pretty consistent. So no real vote for either on my part.

Also I'd like to state what is meant by "constant". I mean, obviously a static const int is, as well as a #define for a pure expression. How about enums? How about integral template parameters? How about immutable values?
I would argue caps for all these cases, since by definition they are constants. You can't change them after declaration and if you do, shame on you! :)

MootPoint
Posts: 58
Joined: Fri Sep 24, 2010 7:44 am

Re: string parsing crashes

Post by MootPoint » Wed Nov 17, 2010 2:57 am

Full words for vars, and kPrefix please. I'm not a fan of ALL_CAPS in code, it's the equivalent of shouting in every other textual paradigm. Why does a const declaration require better visibility than other code? It doesn't...

wololo
Site Admin
Posts: 3727
Joined: Wed Oct 15, 2008 12:42 am
Location: Japan

Re: string parsing crashes

Post by wololo » Thu Nov 18, 2010 2:04 pm

I've updated the rules at:
viewtopic.php?f=15&t=397

I think we still need to agree on constants, let's keep discussing those:
local constants
global constants
enums
macros/defines

do we want them to all be the same? I'd be in favor of yes, all uppercase. does all kStuff make sense? kStuff for a macro sounds weird...

mnguyen
Posts: 229
Joined: Thu Apr 29, 2010 4:13 pm

Re: string parsing crashes

Post by mnguyen » Thu Nov 18, 2010 2:17 pm

having all caps for these does remind me of "shouting" but then again, that also makes sense. You want to be able to differentiate variables that are effectively immutable from others visually as well. Aside from using "m" as a prefix I'm not a huge fan of Hungarian-like notation. This may be my Java-background speaking however. In the case for C++ where global variables already exist (eek) using "g" to differentiate until we can do away with them is good.

My thoughts anyways.

Post Reply