Open KeyWorksRW opened 1 year ago
Note that in some cases we currently have the same function names as wxWidgets (which uses CamelCase) so switching to camelCase will either be helpful to distinguish from a wxWidgets function, or annoying because you got used to camelCase and you were expecting it to work for a wxWidgets method. 😁
PR #1121 changed the Node classes so that they all using consistent function naming. This was by far the worst of the multiple naming conventions within a single class. I'm not as concerned about the rest of the code base, so I removed the high priority label.
Description:
The code base has been under development for several years, and over time different conventions have been used for function names. That can make it a bit confusing to find which convention was used for a specific function. I.e., does the function start with
get_
,get
orGet
? In some cases, two versions are supplied as inGetParent()
andget_parent()
.All of this is made worse when coding is used with AI such as github CoPilot or Tabnine. Load node.h into an editor like VS Code, and the AI won't know whether to use
value()
oras_string()
,GetParent()
orget_parent()
, etc.There already is a little bit of a coding convention:
std::
classes which are lower-case.My inclination is to use camelCase for
get...()
,set...()
andis...()
functions. Theas_...()
functions -- are all used for retrieving property values so leaving them as snake_case is fine. For someone familiar enough with the code base to realize that, it makes readability a bit better since you immediately know what the function is retrieving.Edit
I reduced the scope of the changes -- priority is standardizing on
get...()
,set...()
andis...()
functions.