Naming convention suggestion

Hi. Suppose I have a function that checks that a car has crashed or not.
I should use CheckCarCrashed() CarCrashed() ParseCarCrashed or use "try" or "assert" word?

Code (CSharp):

1. void CheckCarCrashed(){
2.    if(condition){
3.       CarCrashed();
4.    }
5.    else{
6.     //
7.    }
8. }
9.  

 

I like CheckCarCrashed. But I think it is more important to just make it easy for you to understand, and consistent with how you are naming other methods, than it is to conform to a specific convention (unless you work for an organization that enforces a convention).

 

depending on what it returns, isCarCrashed() would possibly require less comments?

 

Out of those three, I like CheckCarCrashed() best, too. CarCrashed() is a statement ("The car crashed."), which wouldn't be accurate for what this function appears to do. ParseCarCrashed() is technical (and not necessarily accurate) without improving readability. Can you come up with a name that tells the reader what the function does if the check is true and/or false?

If you were going to write a function that returns a Boolean value if the car is currently crashed against something, IsCarCrashed() would be good .

 

If its name starts with "is" or "has", then it has to return bool This one is void. Hence it should be "check" or something similar. Same applies if function name sounds like something that can be true of false (carCrashed() ) . If it can be answered with yes/ho, it should return bool. If it doesn't return bool, it should be renamed.

On related note, I prefer to start function names with lowercase.

 

CheckCarCrashed() is best - or CheckForCollision() if that's appropriate, since presumably your script is not so general that you need to specify that it's the car crash you're looking for.

'Try' is a word that goes with an intentional action - for example if you want to fire a weapon only if it has enough charge.

 

Thank you all. So here the word "Check" is the best because it checks that the car has crashed or not.

Code (CSharp):

1.  
2. void Update(){
3.    CheckCarCrashed(); //CheckCarCollision
4. }
5. bool CarCrashed(){
6.    // Use Physics and raycast to check collision
7.    return true;
8.    //
9.    return false;
10. }
11. void CheckCarCrashed(){
12.    if(CarCrashed()){
13.       ExplodeCar();
14.    }
15.    else{
16.     //
17.    }
18. }
19.  

"CarCrashed() is a statement", Completely true so it is good for a function that returns boolean or for enum states.


you said "IsCarCrashed" is good for functions with Boolean return.
if(IsCarCrashed()) better than if(CarCrashed())?

And does anybody use question mode in naming conventions?

Maybe you are a javascript or c++ developer, LoL

Yes thank you.

Code (CSharp):

1.  
2. void TryFireWeapon(){
3.    // or weaponHasCharge
4.    if(hasCharge) {
5.  
6.    }
7. }
8.  

 

And another case suppose we want to find an employee

Someone says "Use context+verb+way" Employee EmployeeFindById(int id). I think if I want to use this function, I need to return Boolean.

Code (CSharp):

1. if(EmployeeFindById(23)){
2.  
3. }
4. or better
5. if(EmployeeIsFoundById(23)){
6.  
7. }

but I prefer write Employee FindEmployeeById(int id) (verb+context+way)
What Do you prefer?

 

EmployeeIsFoundById is just a variation of IsEmployeeFoundById, which should return a bool. (The convention is that Is and Has return bool.)

If you want to return an Employee object, use FindEmployeeById.

Regarding the car crashed thing, I think it would help to have a really clear idea of what the function is supposed to do and what it returns, if anything. Before writing a function, I usually write comments for the input parameters and output/return value. This acts like a contract for anyone using the function.

 

I prefer

Code (CSharp):

1.  
2. void Update(){
3.    if( IsCarCollision() ) ExplodeCar();
4. }
5.  

This has less to do with pure naming convention and more to do with readability.

The above can be read straight up as a sentence. The sentence describes the condition and the result.

Compare this to

Code (csharp):

1.  
2. void Update(){
3.    CheckCarCrashed(); //CheckCarCollision
4. }
5. bool CarCrashed(){
6.    // Use Physics and raycast to check collision
7.    return true;
8.    //
9.    return false;
10. }
11. void CheckCarCrashed(){
12.    if(CarCrashed()){
13.       ExplodeCar();
14.    }
15.    else{
16.     //
17.    }
18. }
19.  

This is much harder to read. "CarCrashed()" and "CheckCarCrashed()" at a glance look quite similar, making them harder to differentiate.

Another problem here is "CheckCarCrash" doesn't just check if the car crash, it also triggers the explosion. Method name should be: "IfCarIsCrashedExplode()" to be accurate.

It's not always bad to have a method like checkcarcrash but the name does not actually describe what the method does.

 

One thing that helped me greatly improve the quality of my code is learning to name things for the reader instead of the writer.

CheckCarCrash for example really describes what you as the writer of the code are thinking about:
"I need to write the code to check the car crash here to trigger the explosion"

The reader doesn't necessarily know that CheckCarCrash leads to the explosion. So CheckCarCrash doesn't mean as much, all it says is that we're going to check for a crash.

As a reader, this becomes confusing when you have bool CarCrashed(). What is the difference between CheckCarCrash and bool CarCrashed?

For the reader, telling them that you are going to ExplodeCar is much more important than telling them you are CheckingCarCrash.

Because you want the reader to know you're going to explode the car, this should be in the method name. If there are multiple results in the check, then other name is appropriate: UpdateCarState for example.

 

IfCarIsCrashedExplode() yes it is accurate but you know if we continue it, it can be a paragraph!
and I think we do not need "is" word in "car is crashed!" sentence. car crashed or car crashes is correct
Suppose I want to explode the car and destroy it after explosion! I have to write "IfCarCrashedExplodeAndDestroy" blah blah

 

Agreed. I tend to prefer to never use past tense in bool methods, but it does make it more correct.

The reason i only use present tense is because having all your bool methods grouped under Is in code complete popup is easier to use. But if "Has" is a convention you're happy with, this is better.

 

It did occur to me after reading over the post, after I had hit submit because that's how it always works, that it didn't really improve the grammar. Guess I didn't delete it fast enough.

Good point about IntelliSense. It also helps when you're editing the file if you don't have a fancy IDE.

 

That's also true, but it's our job as programmer to highlight the most important stuff from less important.

We don't need to name it IfCarCrashedAssignBoolThenLoopAndSwitchCondition...

We know that Explode is the most important part, and because it's the most important part, highlighting it for the reader is valuable.

(I agree with you on Is being unneeded, IfCrashedExplode() gets rid of other unneeded words also - reader should understand we're exploding the car so we can drop the word)

You can always find problems with naming, it is not science, it is art, so hard rules will always have exceptions. The important thing is that it communicates to the reader.

 

Thank you Ryiah. I like it, hasCarCollided() more than didCarCollide() and both more than CarCollided() because it is more clear.


"One thing that helped me greatly improve the quality of my code is learning to name things for the reader instead of the writer."

Thank you Perfect.

I am not an English man but I think car is crashed is not correct right? lol
the correct is:
The car crashed
The car has crashed
The car is crashed by ....
and finally you can not always use "is" for all boolean return functions, Can you? for example HasShield(),HasArmor(),HasGunWithId(int id).

And one thing I think is important is that we should use long names for private classes and methods and instead short names for public classses and methods. It is contrary for fields
Thank you all. Good advices

 

IsShielded() or IsShieldBroken() for the reverse. I strongly prefer to use positive statement like if( IsShieldBroken() ) instead of if( !IsShielded ) where reasonable.

HasGunWithId - I would prefer:
bool TryGetGun( id, out Gun )
or
[CanBeNull]
Gun TryGetGun( id )
If you really want straight bool:
bool IsGunContained( id )


Sometimes it's awkward yes, but if you always use Is then its easy to find stuff with intellasense (especially in monobehaviour which has a billion functions already - so you can't see clean list). I would rather break grammar than make it harder to find stuff w/ intellasense honestly.

 

And another question, I have a problem when I change my names of functions because of refactoring, I missed references in the inspector(like OnClick). So it is prone to not change them.

 

This is why I don't use inspector for events. It always is a nightmare to fix this problem. Plus if you use "Find Usage" it won't show you inspector references.

Wire everything in code and your life will be easier later.

You can use [FormerlySerializedAs] attribute but you need to remember to do this or data gets wiped. I donno if you can apply this attribute to methods or not, but fields it works.

 

I've avoided arguments about what is right or wrong, and avoided any house style casing discussions as they're pretty much not important. Every company or studio will have a different choice there.

What I wanted to focus on with my reply here, is the use of context and the English language, specifically if it will be paired with an If statement and if the tense is past, present or future. These are what will quickly clear the matter up for you by process of elimination. It's IMHO the best non-personal and easiest to understand method as it uses the rules of the English language.

If the crash has already occured...
HasCarCrashed()
IsCarCrashed()

If it doesn't matter if its a car or other vehicle...
VehicleCrashed()

If it needs to calculate if it will crash / is currently crashing
CheckCrash()

As you can see, less is usually more, and you may use the context of the code where the function is typically used plus the return type to reduce verbosity. The English language solves the rest. And the best code here needs no comments unless there's a specific reason why the code has to be this way (thanks @superpig for the comment observation in another thread).

 

And I do have a problem reading this because:
void CheckCarCrashed() does not return anything, yet the English language indicates that the function should really be a bool, because it's checking if the car crashed already, but returning nothing! This is just bad use of the English language, and sends mixed signals about the purpose of the function.

It really should be split into two functions where first you call HasCarCrashed() or IsCarCrashed() and then CalculateCarCrashResult().

Alternatively you'd rename the function to void HandleCarCrashStuff() - which is also bad. There are better solutions to all of these suggestions, if you read my above post. I am not going to dictate what is right or wrong for you, only show how powerful and useful it is to use context, purpose and time to shape your code.

 

To be fair, most of the time we don't care about precision here. IsCrash or IsCrashed will most of the time have the same result. If exact timing matters, its usually better to use a form that makes that clear:

OnCommitting / OnCommitted
OnCommitBegin / OnCommitEnd

Most of the time, as programmers we don't differentiate between present and past tense logically, they should almost always resolve to the same code.

If they don't, then using a name that draws attention to it is worthwhile. Pairing methods up like the above is also helpful. You type: "OnCommit.." and the popup shows you both versions, you immediately know you need to differentiate.

 

No, you can not apply the attribute ([FormerlySerializedAs]) to functions. Yes it is a nightmare but if you use scripts for events like OnClick and eventTriggers, I think it creates coupling as well and also designers can not work with them very well

 

My $.02:

- I like the Is and Has in relation to returning bool.
bool HasGun(), bool HasBullet(), bool IsCollided()
- I would not use the "Car" in the method name
It is because the noun usually the class itself. At least it should be. Unless you are making a class which should be broken up to more granular classes.

Code (CSharp):

1. Vehicle car = new Vehicle();
2. bool isCarCollided = car.IsCollided();

And also:

Code (CSharp):

1. void ExplodeOnCollision () {
2.   if (car.IsCollided()) {
3.       car.Explode();
4.   }
5. }

On the note of HasUser or FindUserById:
- HasUser(id) or IsUserExists(id) -> returns bool if the user with the id exists
- FindUserById(id) -> returns the user data structure if the user with the id exists

I prefer Is or Has because it is not just used with in relation of "if" it also used to assign value:
if (IsUserExists(id)) {}
and
bool userExists = IsUserExists(id);

Although the same rule applies here too, so it's even better on this way:

Code (CSharp):

1. Person adam = new Person();
2. bool doesUserExist = adam.DoesExist();

 

Interesting @LurkingNinjaDev

Thing is though, these names, they're often chosen in isolation. My proposal here is that people stop designing names in isolation, but instead make the name choose itself based on what it is, when it is and what it is for in context. Here are a couple of places where your example conflicts slightly with that, and could be improved (IMHO) just for fun of course:

if (car.IsCollided())

Reads as:

If car is collided

When it could read as:

If car collided

If we continue with this it'll be:

"if Joe has gun" - good
"if gun has bullet" - good
"if bullet is collided" - is bad grammar, should probably be HasCollided

It's just a silly attempt to allow the naming problem to solve and name itself by leveraging English language and grammar.

I'm sure half of my code sounds like a yobbo or street thug though...

 

lol! That's how you know you are actually getting something done. If everything is too neat something is wrong

 

Well, in my day job I develop enterprise applications in Java, so the naming convention is all the gods combined. (Although I have to admit, I myself am among the more lazy ones...)

I think the secret is to not overthink it. English readability is good but aiming for good grammar is overshooting. I believe.

And yes, I absolutely agree that we should not name anything in isolation. That's why I mentioned the object names in my examples and the fact that what I'm saying stands only if we're talking about a proper OOP and not some messy class which contains a lot of stuff.
(In case of game programming I don't mind more messier code: performance and simplicity are everything when it comes to game code, readability is just in the top 5, while in enterprise, when you have 200+ other people reading your code, readability is everything, performance is just second, but my habits from my day job usually leaks to my hobby projects as well)

ROFL
Well, my English grammar is inherently sucks (I'm not a native English speaker), so I wouldn't mind.

Or in code:

Code (CSharp):

1. Person Jules = new PulpFiction.Person();
2. bool doesSpeakEnglish = english.Mo****er().DoYouSpeakIt();

 

LoLL