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

web.cpp cleanup #97

Merged
merged 6 commits into from
Jul 26, 2024
Merged

web.cpp cleanup #97

merged 6 commits into from
Jul 26, 2024

Conversation

Bixkitts
Copy link
Contributor

Just a little janitorial work, I simply like to clean as I read.

I did the following:

  • Broke up some massive 200-400 line functions into smaller ones.
  • Replaced some switches+enums with branchless function arrays for speed and readability.
  • Binary size is SLIGHTLY smaller.

I can undo the weird indentation stuff if you don't like it,
but C++ Core Guidelines encourage liberal use of whitespace for
legibility.

C++ code is still very C-like, could make it more "idiomatic",
but I'm happy with it.
Tested it on our hardware and still seems to do what it should when I click around!
Please double check function arrays (for off-by 1 or wrong indices), and getRootData() - I reordered a lot of code.

If these kind of changes are welcome, I'm happy to do some more.

@xyzroe
Copy link
Owner

xyzroe commented Jul 26, 2024

Wow, thank you for your work. 🚀
This is very impressive!

I'm currently on a short vacation, so I won't be able to review your work right now. I trust your tests are sufficient, and any issues that arise later can be fixed then.

Regarding formatting - I use a plugin that handles indentation. If it doesn't remove your added whitespace, that's fine by me, it's not a big deal.

Any improvements to the code, whether it's fixing bugs or enhancing readability and performance, are always welcome.

Thanks again, and keep up the great work! 🤝

@xyzroe xyzroe merged commit a42b7f4 into xyzroe:main Jul 26, 2024
1 check failed
Copy link
Contributor

Thanks @Bixkitts 🏆

xyzroe added a commit that referenced this pull request Sep 14, 2024
        - Add option to disable CRON firmware check (#108, #104, #99, #88)
- Little mistake in french translation on the status page #107 Thanks @alainvdu69 🏆
- web.cpp cleanup #97 Thanks @Bixkitts 🏆
- Ita translate error #91 Thanks @kirkoff71 🏆
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