string parsing crashes

All code submission.
Zethfox
Posts: 3025
Joined: Thu Jun 10, 2010 11:28 pm

Re: string parsing crashes

Post by Zethfox » Tue Nov 16, 2010 8:52 am

translation:
we made the rules, you follow them, we have excuses for everything you have prsented us with and your anwser is no....



ok, understood :/

transparency is not
ATrbWidth <-----not this

attrubuteWidth.....<--this is tranparency

i dont care how well you program, how many different types of programming you can do. i KNOW that suggesting that the source be made more transperent by removing acronyms, shorthands, ect from the code is a GREAT suggestion.
it boils down to HUMAN eyes reading these shorthands, in my eyes it looks like the person is trying to look like an elitist jerk :/ seriously, you could EASLY add those 3 letter you removed and the code would READ better. ATK<---really, you couldnt type ATTACKER? are you that elite?

anyways, sorry for wasting your time and mine even suggesting such a change.

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

Re: string parsing crashes

Post by mnguyen » Tue Nov 16, 2010 9:04 am

Zethfox wrote:if this is all coding lingo, then atleast a comment explaining exactly what the varible does or function is supposed to do. im seeing wagic is going to be heading this direction of strict coding rules, acronyms, shorthands, ect...those are not transparent. yes you understand all the coding lingo, but you made this project open source....that means your lingo, mikes lingo, moots lingo...you cant slap them in anymore.
i think what i suggest with forbidding acronyms, shorthand, abbrivaitions makes this OPEN SOURCE projects more uniform and understandable by ALL that take part in it. because while you and jack might know what "nb" stands for, im POSITIVE mike and i didnt know.
attrabs vs attrabutes....the difference is that youre using REAL words. transparency my friend. this is after all open source.
understand?


i want to EPICALLY point this out...

Most programmers should now what "lc" stands for.

you see wololo....you see...keyword...MOST......

if you were to name it EXACTLY what it is..."prestringedCardName" whatever it actually means....then the "most" would become "ALL" understand?


also for "readability"
were going as far as putting brackets and braces on their own seperate lines.....
seriously? and you wont even consider what i suggest in making the coding much more uniform for ALL to code in...basically remove those comfy acronyms that YOU might understand and are use to that make NO SENSE to the next guy, a little sense to the next, is understood by that guy, but looks like hyroglyphs to the 4th programmer.
Zeth, I think I have to take that 10 dollars from you as lc is as I've stated, common knowledge amongst programmers. :D As far as "k" goes, if not common knowledge then very easily deciphered. "k" is used in lots of other subjects like math, physics, chemistry, etc to denote a constant. So it's not all completely restricted to coding "lingo" as many things do span across more than one subject.

However, I think I have to disagree with the acronyms part of it. As a Java developer, I do believe in descriptive variable naming. However, that doesn't mean everything has to be spelled out. Anything that is in common usage in a system is a candidate for abbreviation. Part of this is because longer variable names does eventually make code less readable as it takes up more space on the screen. It is also important to understand the system you are coding against before making any decision as to why this abbreviation is not sufficient or bad. Any project, open source or not, requires a prospective developer to spend time understanding what the code is doing. Understanding everything before coding is difficult and not always possible, but knowing what are the most common objects are will help one understand the acronyms. For instance, it really makes no difference to a new developer to the system if the variable is called "tc" or "targetChooser" as he/she will have no concept as to what a TargetChooser is. However, after spending time in the code it becomes fairly evident that tc is most likely "TargetChooser".

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

Re: string parsing crashes

Post by Zethfox » Tue Nov 16, 2010 9:07 am

ok even moot had no idea what "tc" stood for as a comment left behind when he refactored cost proved, so its not THAT common knowledge,
you guys can go ahead and drop this convo, its one of those "we're going to discuss this but all signs point to your wrong and we're right"
AKA "we made the rules, you will follow them, and youre wrong".....

btw

atk

attacker?

yeah which is harder to read ?

also

tc is most likely "TargetChooser". <-----i like how you and wololo just fall into this trap constantly....
most likely.....as in not 100% sure?...yeah i think thats what you mean by "most likely"

Most programmers should now what "lc" stands for. <---most.....again....most is not 100%.

if i stab you 37 times...and i sew up "most" of the wounds...you will die. its that simple to understand.
most is NEVER 100%.
thats my point...thanks.


on another note this needs to be changed then

As far as "k" goes, if not common knowledge then very easily deciphered. "k" is used in lots of other subjects like math, physics, chemistry, etc to denote a constant

in wagic
ALL constants do NOT use this letter "K" infront of their words....only YOUR constants add this letter in the front.
YOU intruduced this. that means your code is inconsistent with current wagic coding.

either you need to remove your "k"s...or add "k" too ALL the constants in wagic.
otherwise this is the start of some constants having k, some dont, then some more get added with K...later on down the road, people will end up confused as to "why this constants isnt listed with a K in front, is it really a constant?"

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

Re: string parsing crashes

Post by mnguyen » Tue Nov 16, 2010 9:27 am

Zethfox wrote:ok even moot had no idea what "tc" stood for as a comment left behind when he refactored cost proved, so its not THAT common knowledge,
you guys can go ahead and drop this convo, its one of those "we're going to discuss this but all signs point to your wrong and we're right"
AKA "we made the rules, you will follow them, and youre wrong".....

btw

atk

attacker?

yeah which is harder to read ?

also

tc is most likely "TargetChooser". <-----i like how you and wololo just fall into this trap constantly....
most likely.....as in not 100% sure?...yeah i think thats what you mean by "most likely"

Most programmers should now what "lc" stands for. <---most.....again....most is not 100%.

if i stab you 37 times...and i sew up "most" of the wounds...you will die. its that simple to understand.
most is NEVER 100%.
thats my point...thanks.

on another note this needs to be changed then

As far as "k" goes, if not common knowledge then very easily deciphered. "k" is used in lots of other subjects like math, physics, chemistry, etc to denote a constant

in wagic
ALL constants do NOT use this letter "K" infront of their words....only YOUR constants add this letter in the front.
YOU intruduced this. that means your code is inconsistent with current wagic coding.
actually "k" was code given to me by somebody else on the team. I just wasn't sure if it already existed. I assumed it was since I didn't come up with it. I'm more than happy to rename it. Things should be more consistent and I agree that "k" is not consistent.

I'm not trying to say "I'm right, you are wrong" I am giving you real explanations as to how things were written. As I've said before this group is very diverse, there are things some people agree upon and others do not. However, this doesn't mean that we can't all learn from one another or at least come to agree to disagree for the time being. I am not a fan of 1 letter variables for anything. I do not enjoy 2 or 3 letter ones either, but I do understand why other people prefer to use them and how they can make sense in certain situations.

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

Re: string parsing crashes

Post by Zethfox » Tue Nov 16, 2010 9:33 am

i see 2 veterans logged in silently watching.
its a shame theyre not commenting.

Users browsing this forum: Jean, wololo

oh well, as i said, its dropped, i will think twice before EVER suggesting such a thing. it was my mistake to think that this project wanted univarsal transparency in the code.

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 9:43 am

Zethfox, Did you actually read the replies or are you just angry for no reason?
Zethfox wrote:ok even moot had no idea what "tc" stood for as a comment left behind when he refactored cost proved, so its not THAT common knowledge,
you guys can go ahead and drop this convo, its one of those "we're going to discuss this but all signs point to your wrong and we're right"
Fair enough. I still think "tc" is a nice abbreviation for a variable so widely used.
atk

attacker?

yeah which is harder to read ?
You mentioned this one already and nobody disagreed with you, what is your point ?
As far as "k" goes, if not common knowledge then very easily deciphered. "k" is used in lots of other subjects like math, physics, chemistry, etc to denote a constant

in wagic
ALL constants do NOT use this letter "K" infront of their words....only YOUR constants add this letter in the front.
YOU intruduced this. that means your code is inconsistent with current wagic coding.
We don't have a rule for constants yet. As a matter of fact, if you read our rules, you will find them here: viewtopic.php?f=15&t=397
Notice how there is nothing about constants.

The reason we are introducing new rules is because the code in Wagic is currently inconsistent.
We are all discussing "what could we do to make it more consistent?", and your answer is "why did you make it inconsistent in the first place?", which doesn't help much, don't you think?

I would personally rather use uppercase for constants, so it's one point we still need to discuss.

Regarding prefixes, you can have a look at other Open source projects, or in wikpedia ( http://en.wikipedia.org/wiki/Hungarian_ ... n#Examples ) and make an idea for yourself.
What initially feels natural to you (full English names) can become very tedious when you have to code, and only experience will tell you what you like the most.
Tell me, what do you prefer:

Code: Select all

if a is greater than or equal to b then call function Myfunction with parameter a else call function MyFunction with parameter b
or

Code: Select all

if (a >= b)
  Myfunction(a)
else
   Myfunction(b)
This is of course a very far fetched example, but just to tell you that "long readable names" are not always what you want.

And just so you know, "Open Source" doesn't mean "people with no experience can read the code". Wagic is quite an exception for that actually, and it certainly doesn't mean that we should have rules that please people with less experience.

a few samples from other Open Source projects:

Webkit:
Use full words, except in the rare case where an abbreviation would be more canonical and easier to understand.
Right:

size_t characterSize;
size_t length;
short tabIndex; // more canonical

Wrong:

size_t charSize;
size_t len;
short tabulationIndex; // bizarre
scummVM:
Naming

Constants

Basically, you have two choices (this has historical reasons, and we probably should try to unify this one day):

1. Camel case with prefix 'k':

kSomeKludgyConstantName

2. All upper case, with words separated by underscores (no leading/trailing underscores):

SOME_KLUDGY_CONSTANT_NAME

(As a side remark, we recommend avoiding #define for creating constants, because these can lead to weird and difficult to track down conflicts. Instead use enums or the const keyword.)

Type names

Camel case starting with upper case.

class MyClass { /* ... */ };
struct MyStruct { /* ... */ };
typedef int MyInt;

Class member variables

Prefixed with '_' and in camel case (Yo! no underscore separators), starting with lowercase.

char *_someVariableName;

Class methods

Camel case, starting with lowercase.

void thisIsMyFancyMethod();

Local variables

Use camel case (Yo! no underscore separators), starting with lowercase.

char *someVariableName;

Note that for POD structures it is fine to use this rule too.

Global variables

In general you should avoid global variables, but if it can't be avoided, use 'g_' as prefix, camel case, and start with lowercase

int g_someGlobaleVariable;

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 9:44 am

Zethfox wrote:i see 2 veterans logged in silently watching.
its a shame theyre not commenting.
Took me 1h to read, comment, scratch my comment because you changed your post, then re-write my reply.

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

Re: string parsing crashes

Post by MootPoint » Tue Nov 16, 2010 9:52 am

some widespread standards in coding:

kSomething means that it's a constant value, ie
const int kSomething = 42;

sSomething means that it's a staticly defined variable, ie
static Foo sSomething;

mSomething means that it's a member variable of a class, ie

Code: Select all

class Foo
{
public:
int mSomething;
}
This is standard fare stuff. The idea is that the prefix letter gives you contextual information that helps understand its usage. I suggest getting used to it, because you'll run into it over & over in the software world.

I'm particularly adamant about using the 'm' prefix in front of class member variables. Let's take the example of a 'card' for instance - how often do you get into a function where there's a 'card' declared locally? Bet you've seen a function or two that takes a '_card' as a parameter (shudder) in order to distinguish it from a 'card' member. As I'm going through functions, I'll hit a variable like 'card' and I need to constantly check whether it was declared locally or as a member. This is incremental mental fatigue that can be entirely avoided by an easy, standardized naming convention.

(On a slight tangent: old school crepes: once upon a time, it was thought smart to prefix variables with a letter representing their type,
ie bBoolean, iSomeInteger, wAWordValue, etc. This is largely useless in object oriented languages and IMO should be entirely avoided. )


On acronyms: I really don't like acronyms. I especially don't like one letter acronym pointers to objects. Get 3 or 4 of these in a body of a function, things get confusing fast. tc should be targetChooser, mq = managedQuad, etc. If you're not familiar with a given library, you'll constantly be scanning back up to where a variable is defined to look at the class it represents. If you explicitly write out a variable name, it's clear. Code is meant to be human legible. If you want code to be more maintainable, the more human readable it is, the better chance someone new has to pick up the code's intent without busting things left right & center.

Now, there's a reasonable balance - I'm fine with things like 'iter' as short form for iterator, because it's pretty obvious and universal. But... what's a tq again? oh, tracked quad... etc. You get my point - it's only obvious to someone who's familiar with the code.

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

Re: string parsing crashes

Post by Jean » Tue Nov 16, 2010 10:45 am

Whoa there. Let's put our minds at rest.

We have a number of people trying their best. Everyone is making every effort to have readable code.

First. A number of things you mention are not avoidable. For example, I may be the culprit for the "Defenser"s around, or wololo may be. English is not our native language, and for my own, I haven't done a single actual game of Magic in my entire life, and all the words in the game were new to me, and at that time "defender" sounded every bit as good as "blocker". As for the mistake, we do our best to speak as well as we can ; that is still a great effort on our part, and it is not something we owe you. We are making efforts on our part. Please be kind on your part and try to understand. When you see a word like "defenser", just correct it, or point it out, or both. Whoever wrote it will be happy to have their mistake fixed.
Now if it is used to mean a creature being blocked, then that's terrible naming, but I'm positive I did never name "defenser" a variable storing a creature being blocked. Now, if someone did... well, see it, sigh, and fix it. If it's been done, it's obviously not on purpose.

Second. Please understand there are extremely often used abbreviations that you are supposed to learn. First, there are prefixes. "m" in front of a member variable name is extremely common - it stands for "member", as g in front of globals stands for "global". "k" is rather common for constants. It is also common use, especially in C and other languages without namespaces, to prefix most identifiers with something consistent used throughout a unit of code. JGE and wagic were originally written in C, and there is still some legacy in this. Also, when you see "wEvent" for wagic event, the intent is to distinguish it from some kind of general event that could be supplied by a framework. It makes sense to prefix wagic stuff with "w", though I do agree with you that it makes little sense to use it only on some parts of the code... we all code our part, on our free time, we don't have time to consult each other on every line of code or to do a full review of the entire source. We do our best.
Then, common abbreviations. "nb" is extremely common for a number. "i18n" is commonplace for "internationalization". "root" is classic for "superuser". "Keyb" is keyboard for just about the whole world. And then when it's the 200th time you spell out "lowercase", you start writing it "lc" because it starts getting obvious. We do try to abbreviate most commonly used words in wagic, because though we want explicit identifiers, this is actually a balance to keep. For the active attacker, "acAtk" is not understandable ; "activeAttacker" sounds good ; "currentlyActiveAndFocusedAttackerPermanentWhichIsDisplayedOnTheLeftOfTheScreen" is ridiculous. But in any case, experience may shorten names, because you're so used to a number of things that you will find more things obvious.

As for the code you posted, honestly, if you find that hard to read, I wonder how you would have written it. It says quite literally "for all attackers, if it's the card in this event, then remove it from the attacker list and trash it". It could use a comment or a macro to make more explicit why it's testing both the card and its 'previous' pointer ; aside from that... I think it's hard to write this in a more simple manner. And if you can and you have the time, please do.

So for abbreviations, we all try to apply our best judgment. It is good practice to keep variable lifespan as short as possible. When they are alive for three lines, a two-character name is perfectly okay.

Code: Select all

{
   /* code */
  for (int i = 0; i < choosers.length; ++i) {
    TargetChooser *tc = choosers[i];
    tc.choose();
    reset(tc);
   }
}
...this looks perfectly okay. It's better, even, than giving the variable a 15 character name. If the variable is alive for a 20-line function, it is not the same, and "tc" is not okay. For a member, it is definitely not okay at all.


As for "atk", I'm sorry but I don't see that as a huge problem as you seem to think. If you have an atk opposed to a blk in the context of a damage resolve phase, I don't see how they could be misunderstood. Besides, if that really bothers you, I don't think anyone would object to you renaming that into "attacker" and commit that.
Remember that variables have a scope and a context. If a boolean is named "stuffHappened" it probably means it is there to indicate that stuff happened. It does not have to say what happened, because maybe all the code is concerned about is that stuff happened or not - suppose it's code that should refresh the display if stuff happened. It's not a *good* name - a better name would be "shouldRefreshDisplay" for example, or I don't know. But remember variables have a role in some algorithm, and sometimes you can't understand what they do without understanding the algorithm - it's just not possible to always name all variables in a way that will make their meaning instantly obvious. If you write a simulated annealing algorithm, you'll have a "temperature" variable, and there is no way you will understand that as long as you don't have a grasp of the algorithm. However, it's not bad naming. Sometimes, it can't be 100% obvious.

In the end, just keep cool. Variables are named like they are because the ones who wrote the code thought it was good at the time they wrote it. If you think the names are too short to be understandable, nobody will object your making them longer, better, more understandable.

It's not easy to work all together. We all have different experiences, languages, sensibilities, and it results in everyone thinking differently of what is easy and what is hard to understand. But we all do our best. Readable code is a common goal of ours, let's try to be as understanding as possible.

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

Re: string parsing crashes

Post by Jean » Tue Nov 16, 2010 10:46 am

Zethfox wrote:i see 2 veterans logged in silently watching.
its a shame theyre not commenting.

Users browsing this forum: Jean, wololo

oh well, as i said, its dropped, i will think twice before EVER suggesting such a thing. it was my mistake to think that this project wanted univarsal transparency in the code.
It takes time writing something that makes some sense. Plus, writing from work doesn't do wonder for available time.

Post Reply