Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code Formatting: Use enums where able; C-style namespace them properly #31

Open
ElectricRCAircraftGuy opened this issue Apr 13, 2020 · 5 comments

Comments

@ElectricRCAircraftGuy
Copy link
Collaborator

ElectricRCAircraftGuy commented Apr 13, 2020

This is a sub-issue of #11. Don't close #11 until all sub-issues are resolved.

Use enums wherever they make sense instead of constants or #defines. Also, namespace them by putting their categorical name before their specific name, separated by an underscore.

Instead of:

enum main_states : byte
{
  STBY_STATE,
  BREATH_STATE,
  MENU_STATE
};

Do:

Use 3 slashes /// to signify "doxygen" style comments above members (good practice even if you don't generate doxygen). See more examples in my file here: https://github.com/ElectricRCAircraftGuy/eRCaGuy_dotfiles/blob/master/git%20%26%20Linux%20cmds%2C%20help%2C%20tips%20%26%20tricks%20-%20Gabriel.txt

enum main_state : byte
{
  // Use explicit name, and explicitly set 1st element to 0
  /// some useful description
  MAIN_STATE_STANDBY = 0, 
  /// some useful description
  MAIN_STATE_BREATH,
  /// some useful description
  MAIN_STATE_MENU, 
  /// Not a state! Use this if you need the total number of enums in this type
  MAIN_STATE_COUNT, // trailing comma on last elment is just fine
};
@ElectricRCAircraftGuy
Copy link
Collaborator Author

I will help with this. Just want to teach the principle too is all.

@ElectricRCAircraftGuy ElectricRCAircraftGuy changed the title Use enums where able; C-style namespace them properly Code Formatting: Use enums where able; C-style namespace them properly Apr 13, 2020
@nimrod46
Copy link
Member

Just so you know, enums are type definition, not a list, so the name should be main_state for instance

@ElectricRCAircraftGuy
Copy link
Collaborator Author

Fixed description.

@ElectricRCAircraftGuy
Copy link
Collaborator Author

ElectricRCAircraftGuy commented Apr 13, 2020

@nimrod46, I'm also accustomed to using typedefs when writing C, and many organizations require enum classes when using C++, but I think the C enum is just fine, but since Arduino is technically C++, it allows a simplified syntax without typedef.

In C I'd add _t to the end of the enum type. Ex:

typedef enum main_state_e
{
  // Use explicit name, and explicitly set 1st element to 0
  /// some useful description
  MAIN_STATE_STANDBY = 0, 
  /// some useful description
  MAIN_STATE_BREATH,
  /// some useful description
  MAIN_STATE_MENU, 
  /// Not a state! Use this if you need the total number of enums in this type
  MAIN_STATE_COUNT, // trailing comma on last elment is just fine
} main_state_t;

Then

main_state_t main_state = MAIN_STATE_STANDBY;

I think we should add the _t if too to specify it's a type, but since we don't need the typedef in C++, I'm fine without it too. @nimrod46 , let me know if you have a preference. Otherwise, I'll plan on going with the _t as well so we can do this:

main_state_t main_state = MAIN_STATE_STANDBY;

...instead of this:

main_state my_main_state = MAIN_STATE_STANDBY;

Notice the problem of having to figure out what to name the name_state variable now in the latter case, since the variable name can't be the same as the type name. Adding _t to the end of the type solves this silly little dilemma. :)

@nimrod46
Copy link
Member

@ElectricRCAircraftGuy I prefer without the _t the current main state should be current_main_state .
I think its the best practice to name it like that anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants