Should we continue using Hungarian notation?

[Pirulax]

In the past there have been discussions regarding Hungarian notation, and that it's usage isn't strictly necessary, but its unresolved whenever we should continue using it in the future.

In the style guide it states, that it should be used.

Hungarian notation in variables

I personally feel like we should stop using it, as its redundant, and when something needs to be refactored it introduces a lot of unnecessary code changes, which makes it harder to review code if a variable is used in a lot of places.

Eg.: changing the underlaying type from unsigned int to float would require us to rename the variable to have a prefix f instead of ui.

The other aspect is that its very inconsistent. What do we do with custom types? Using a prefix of S doesnt tell anything about it.

(Edit) A good example would be std::vector. Sometimes contributors use vec prefix for them, whereas vec should only be used for Vector2/3/4. What do we do with container types? std::list, vector, map, etc.. I tend to use m_PlayerList for both vector and list, because m_PlayerVector sounds misleading (as in Vector position)

Hungarian notation in typenames

For structs/classes we use the S and C prefixes. It makes Sort by name in File explorer completely unusable, because the first character is always C. (or its omitted)

While I think these prefixes are mostly useless by now (as we shoudnt be using unions anyways, they're replaced by std::variant), I think we should keep them so the code remains a little more consistent.

Theres are no differences semantically between class and sturct, while, there are between the latter and a union, and mind you, we use the same prefix for both, which is misleading at best.

I think we can stop using this as well, its useless, as theres no difference semantically between a struct and class.
For unions we tend to use S prefix as well, while its not a struct, so again, its inconsistent.

How was Hungarian notation intended to be used?

Not like we do. MS provides very big APIs, where using Hungarian notation made a self documenting API, with type prefixes like OUT, and IN.

We dont provide an API, and our code, in contrast to M$'s, is open source. We have powerful IDE's which werent a thing in 70'
-80', when this was invented.

Should we continue using it?

Continuing to use a style that is inconsistent in itself, while it introduces a no benefit at all for us makes no sense. We better just be consistently inconsistent by not using hungarian notation at all, which leads to more consistent code in itself.

Probably not

As discussed earlier Im very much in favor of dropping it altogether stopping to use it, but, its not my decision of course, but Im pretty sure all of us wants to drop it, and if its used by anyone its so the code is consistent with old code, but, again, as discussed, most of the time hungarian notation isnt consistent in itself

What should we keep?

Non type prefixes like: m_, s_, ms_. They're very useful.

[qaisjp]

My personal opinion:

• For members, consistency with the surrounding class is important/
• I find Hungarian notation kinda nice and useful when reading long prose code, where being careful about the behaviour between types matters.
• I haven't thought about dropping the S, C or [eE] prefix for typenames. My personal preference is not to drop it.
• Having very short (3 line) functions that don't use Hungarian notation is OK.

If we migrate, my personal opinion is to do it on a project-by-project basis. E.g. starting with only Lua definitions.

I don't think we can ever put in the effort to migrate everything, and I don't see much value (or code safety) in migrating everything. So I don't think it's a good idea to change to something else, because our code will be 50% Hungarian and 50% the other. I think right now 95%+ is Hungarian. Inconsistent code is not very nice.

I think we can stop using this as well, its useless, as theres no difference semantically between a struct and class.
For unions we tend to use S prefix as well, while its not a struct, so again, its inconsistent.

I think our practical uses of unions makes this a non-issue.

Continuing to use a style that is inconsistent in itself, while it introduces a no benefit at all for us makes no sense. We better just be consistently inconsistent by not using hungarian notation at all, which leads to more consistent code in itself.

Other than unions, how is our use of Hungarian notation inconsistent?

[Pirulax]

Updated.

Also, I dont think we should migrate old code. Thats a lot of unnecessary work put into something that has no end-user benefit. If we do decide to drop hungarian notation, we should write code without it, while the old code would remain unchanged.

[Pirulax]

On Discord, @qaisjp said:

Hungarian notation helps you read code because you don't need to commit the type to memory

As mentioned above, we have powerful IDE's whom take care of this for us.
Do you really care about what type m_Position is? Its probably a Vector, and not a bool.
m_CountOfApples is probably an integer type as well, and its probably not negative.
m_LastVersion is trickier (CMtaVersion), but we dont have a type prefix for it anyways.

Only place where I would find it useful is game_sa, where you really need to keep track of variable types.

I think our practical uses of unions makes this a non-issue.

Mixing struct's and classes is fine, but mixing the former and union isn't as much fun.

[TheNormalnij]

I think we should not mix few notation in one project. Code with few notations looks bad and unprofessional.

m_CountOfApples is probably an integer type as well, and its probably not negative.

Sometimes we use char type in gta sa struct's and we should see where do we need a conversion.

[Pirulax]

We wont be using any notations, so theres no mixing.

Code with few notations looks bad and unprofessional.

What do you mean?

Sometimes we use char type in gta sa struct's and we should see where do we need a conversion.**

Thats why I said we should keep it in game_sa where it actually makes sense.

[TheNormalnij]

We wont be using any notations, so theres no mixing.

No notations is also notation, imho.

[qaisjp]

What do you mean?

I believe he means "Code with multiple notations looks bad and unprofessional", and I am inclined to agree.

[Pirulax]

Yes, but, having code where we dont consistently use prefixes (because there are no viable prefixes for some types) isn't better either imho:

A good example would be std::vector. Sometimes contributors use vec prefix for them, whereas vec should only be used for Vector2/3/4. What do we do with container types? std::list, vector, map, etc.. I tend to use m_PlayerList for both vector and list, because m_PlayerVector sounds misleading (as in Vector position)

[qaisjp]

Sounds like our current notation for that is to do m_PlayerList etc?

[Pirulax]

yes, and thats how its supposed to be.

for example, CVector vecPlayerPos is bad. Why? You have redundancy in it: pos can't be anything else other than cvector (or cvector2, whatever). Also vec doesnt tell its exact type, so what have we achieved?

[qaisjp]

What type is trainPos? Is it a vector or is it a number? Note that getTrainPosition is a scalar distance across a track.

[Pirulax]

You'll probably mistype it once as CVector, then realize its not (due to intellisense), and use float, still better than having to type f, and vec prefixes all the time.

[tederis]

Programming is not about types itself. It's more about operations with them.

A simple example:
auto position = origin + velocity*dt;

Logically there is no difference between CVector + CVector and float + float, because in this example the only operators are important.

Another example:
if (now - startTime > 1000) {}

Again, the only operation is important here.

The most of programming constructions is self-explanatory. Otherwise they should be redone to become self-explanatory. In case we choose a right way to express our intention in code a type notation is tend to be redudant.

[Pirulax]

I agree with you. This is what I was trying to explain.

[Addlibs]

What about reading code outside an IDE like on Github directly? Programming isn't all writing, it's often reading. The current notations definitely help imo. And when it comes to refactoring / changing types, modern IDEs let you rename variables flawlessly, so there's no problems there iyam.

[Pirulax]

Yeah, in an ideal world h.n. would help you out when reading code. But currently, its really hit or miss. Sometimes vec is std::vector or CVector. Arrays might be arr, a or nothing.
Bad documentation is worse than no documentation.

And when it comes to refactoring / changing types, modern IDEs let you rename variables flawlessly, so there's no problems there iyam.

As stated before, the problem is that is introduces a lot of unnecessary code changes, which spams git blame, and makes code review harder.

[qaisjp]

FWIW all of the code I write today (outside mtasa-blue) doesn't use Hungarian notation and there's been no problem reading it.

I can support getting rid of Hungarian notation, I think the biggest problem is consistency with existing code. What rules do you propose we use when deciding whether to have style consistent with the surroundings?

[Pirulax]

We should keep hungarian notation in game_sa, and mp_sa, because the reversed already contains hungarian notation, and getting rid of it would be extra work. As for the other projects, I feel like, unless modifying an existing class, we shoudnt use h.n. at all.

[AlexTMjugador]

My two cents on this discussion:

I agree that Hungarian notation by itself does not make the code much more clearer or easier to understand. Like @tederis said, if in a project like MTA the variables are named in a way that it is not possible to guess what data type they are within a given context, I'd inclined to say that the variables or the code themselves are to be blamed (because the variable is either something like a void pointer that gets casted to whatever is needed, or its name or usage is not meaningful). However, there is one exception to this, as others have already pointed: interfacing with GTA: SA code, because that gets ugly quick and Hungarian notation may help making some sense out of it. So I approve the idea of getting rid of it for the most part.

However, I also think that mixing notation (or un-notation) styles will also make the codebase look somewhat divided or messy, and that MTA development resources are better spent on other matters that have more impact on the mod. Sure, Hungarian notation may imply writing a few more characters for each variable name you use on new code, or scratching your head a bit with what would be the correct prefix (which may be good, because it makes you think about your datatypes, and maybe realize you did something wrong in the process), but refactoring the type of a variable would also probably imply refactors to the code that uses it (does this really happen so frequently, even?), so I don't think the extra overhead of Hungarian notation would hinder Git blame and MTA development in a noticeable way. I see it more like a harmless relic that remembers how programmer practices have envolved through the years, so I would just keep it as is and move on.

[Pirulax]

What I meant is that, for example I refactor a class and theres a variable named "m_uiMoney", and I want to change its type to int: Renaming the variable may - depending on how heavily its used - may affect a lot of code, which can end up messing with git blame, and not only that, but its harder to review imho.

[AlexTMjugador]

What I meant is that, for example I refactor a class and theres a variable named "m_uiMoney", and I want to change its type to int: Renaming the variable may - depending on how heavily its used - may affect a lot of code, which can end up messing with git blame, and not only that, but its harder to review imho.

Yeah, I get your point. But when doing such changes you should review the code that uses the variable anyway, and probably make some changes. Continuing your example, switching a unsigned integer to a signed integer may break code that relies in subtle signedness differences, like overflow to a minimum value. It can be said, then, that Hungarian notation helps a bit by bringing more eyes on the lines of code that use the variable, so I don't think that this particular point is a strong reason for ditching Hungarian notation.

[Pirulax]

Good point for sure.
But the compiler would warn you about it probably, thats why we should try to make MTA more warning free, because right now its cluttered with warnings, and nobody cares looking at them, so a warning like this would go unnoticed for sure.

[ghost]

I personally do not like Hungarian notation. Modern IDEs are super advanced, you can easily figure out the type by hovering your mouse cursor over the variable. I can't think of any case where Hungarian notation can be helpful. It does more harm than good. Right now, we know that the MTA client will only compile on x86 MSVC, but maybe in the future, like 10 years later, we have GTA:SA fully reversed, and then porting the x86 code to x64 will look very stupid since we hardcoded the type names in the variables.

Some people use it for function arguments to clarify the in/out usage, like CVector* pOutPosition. This can be avoided:

// This tells us position will be modified in the function
void function(CVector& position)

// This tells us position is readonly
void function(const CVector& position)

[qaisjp]

Does this discussion include dropping m_?

[ghost]

Does this discussion include dropping m_?

No, that's something I can get behind. writing this-> to access members is kinda weird to me. :)

[Pirulax]

No, as said ms_ would stay, I personally find it the best prefix for member variables. Using this-> or _ (underscore before the name) is very weird.

[ghost]

I think we should only use C++ features if they improve code readability or unless we have a very good reason for using them. The argument parser is a good example of the proper use of C++ features. I don't see any problem with

bool function(const CVector& position);

It's simple, readable, maintainable, and gets the job done. Imagine throwing std::pair or std::tuple everywhere, sounds like a nightmare. It might be good in edge cases, but not in most cases.

[Pirulax]

std::optional

[Pirulax]

Anyways, I don't want to go off topic, since this isn't related to hungarian notation anymore.