News:

Simutrans Wiki Manual
The official on-line manual for Simutrans. Read and contribute.

Use of player/player_/p in class arguments

Started by ACarlotti, January 16, 2019, 05:31:33 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

ACarlotti

I've been going through the changes made in r7453 (translating spieler->player) to fix any mistakes or inconsistencies in how this was merged into Extended. I've discovered that there is a lot of inconsistency over whether a method argument is named "player" or "player_". It might be relevant that sometimes "player" is also a member of the class (often by inheritance); however, there is no clear link between this and the choice of "player" or "player_" for an argument name. Even within a single class both "player" and "player_" are sometimes used as argument names. In one case this issue is averted by using "player_builder" as the class member name.

I mention this because in addition to the inconsistency within Standard, there is also a great deal of inconsistency between Extended and Standard (including one place where Extended has used "p" which is more in keeping with the other parameters of that method). So, are there any thoughts on what would be better? I would like to make Standard an Extended use consistent naming, and I think it would be better to choose a sensible consistent naming scheme and adapt both Standard and Extended to this scheme, rather than changing Extended from one inconsistent scheme to another.

Ters

When a member function takes a parameter that has the same name as a member variable, there are some possible cases I know of:

       
  • The function assigns the parameter to the member variable: Call the parameter new_player, or p, or similar.
  • The member variable and the parameter serve different roles: Rename them to represent the roles, such as owner, buyer, seller, user or similar.
  • Someone forgot about the member variable when adding the parameter, or the member variable was added after the parameter: Remove superfluous parameter.

ACarlotti

Quote from: Ters on January 16, 2019, 07:24:46 PMWhen a member function takes a parameter that has the same name as a member variable, there are some possible cases I know of:
The function assigns the parameter to the member variable: Call the parameter new_player, or p, or similar.
I think there's a (related) fourth case, in which your suggestions don't necessarily work. In the case of a constructor of a member variable named 'owner', using the form "new_owner" would seem odd because it suggests that there was also a old value for owner. The obvious abbreviation 'o' is also a bad idea. I think in these sorts of cases appending an underscore (e.g. 'owner_'), which is already in use in many places, seems like a good idea, particularly when it's only used in an initialisation list.

Ters

Quote from: ACarlotti on January 22, 2019, 09:54:57 PM
I think there's a (related) fourth case, in which your suggestions don't necessarily work. In the case of a constructor of a member variable named 'owner', using the form "new_owner" would seem odd because it suggests that there was also a old value for owner. The obvious abbreviation 'o' is also a bad idea. I think in these sorts of cases appending an underscore (e.g. 'owner_'), which is already in use in many places, seems like a good idea, particularly when it's only used in an initialisation list.
In these cases, there is often no need to use a different name, since they are the same.

ACarlotti

Quote from: Ters on January 23, 2019, 04:45:40 PMIn these cases, there is often no need to use a different name, since they are the same.
I suppose so, although isn't there a risk of the parameter being modified subsequently instead of the object member? I don't know if that's actually an issue anywhere but it seems like something that could happen accidentally in a large constructor.

Ters

Quote from: ACarlotti on January 23, 2019, 05:14:21 PM
I suppose so, although isn't there a risk of the parameter being modified subsequently instead of the object member? I don't know if that's actually an issue anywhere but it seems like something that could happen accidentally in a large constructor.
That is why I put in the word "often".

I've written many classes so far this year that accepted a parameter which it puts into a field with the same name, and then the constructor uses the parameter rather than the field for other logic. In these cases, it doesn't matter, since the objects are passed by reference and the field is a reference (and I don't make copies explicitly). Hence it is the same object. These classes also have setters which take in parameters with the same name as the field. In both cases, it is primarily because my IDE auto-generates them that way, and I just run with it.

If a class takes in a value in its constructor, stores it in a field, and then mutates them differently, then I think it should be possible to come up with better names for one or both, whether it is simply "...template", "initial...", "original...", or something else completely.