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

Use enums for Modes and RawModes in C #8510

Open
wants to merge 59 commits into
base: main
Choose a base branch
from

Conversation

Yay295
Copy link
Contributor

@Yay295 Yay295 commented Oct 28, 2024

Adds some type safety when comparing modes, and replaces a lot of string comparisons with enum comparisons.


typedef struct {
const char *const name;
} ModeData;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've created a struct with only one member, presumably because you expect to add more members in the future. Are you trying to prepare for #6547?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some things I think could be added to this struct in the future, but the more important reason for this as it is right now is type safety. A char * could be almost anything, but a ModeData can only be a ModeData.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong, but the only place where ModeData is returned is from Mode.c's getModeData() (and similarly, RawModeData from getRawModeData())? It's used there, and in the construction of the MODES array in Mode.c, and by design, never anywhere else? I don't think we even pass between ModeData between functions - every use outside of Mode.c is just to access the name.

So the only benefit is to assure us that getModeData() and getRawModeData() return the correct output? This could just be my opinion, but I think they are simple functions, and it's overkill to be passing around a struct to the rest of the C code to ensure their correctness.

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

Successfully merging this pull request may close these issues.

2 participants