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

Try to improve rust code readability #219

Merged
merged 2 commits into from
Sep 23, 2024
Merged

Try to improve rust code readability #219

merged 2 commits into from
Sep 23, 2024

Conversation

Lol3rrr
Copy link
Contributor

@Lol3rrr Lol3rrr commented Sep 20, 2024

As suggested in issue #218

Type of changes

  • Try to flatten code by using early returns/continues if there is only a single happy path in a function or loop
  • Change repetitive matches with map_err
  • Change matches returning true/false with matches! macro

Reduce deep nesting in certain cases, where there was only a single happy path anyway
to make the code easier to read and less confusing.

Try to improve error handling by switch elaborate matches with map_err and ?-operator
@Lol3rrr
Copy link
Contributor Author

Lol3rrr commented Sep 20, 2024

I was also thinking about replacing the use of lazy_static with the new LazyLock or similar features in the standard library to remove the dependency, but I was not sure about the supported/used compiler versions

@LaihoE
Copy link
Owner

LaihoE commented Sep 21, 2024

Looks good! Sure we can replace lazy_static also but maybe make it a separate pr.

@LaihoE
Copy link
Owner

LaihoE commented Sep 21, 2024

As for the compiler version, It would be good to set the version in the Cargo.toml as protobuf often also has issues with some versions.

Shortened some more struct constructors from 'field: field' to just 'field'.
Change some usage from '&mut Vec<>' to '&mut []' as recommended by rust itself, if
we are not using any Vec specific api
@Lol3rrr
Copy link
Contributor Author

Lol3rrr commented Sep 21, 2024

I think this is most of the easy to spot things, there are some other parts that seem more complicated and I don't think this would be the right place to change them.

I will open another PR for removing lazy_static and maybe another one to see what clippy would suggest

@LaihoE
Copy link
Owner

LaihoE commented Sep 23, 2024

This contains quite a lot of code already, are you ready to merge this? you can always open a new pr with more refactors.

@Lol3rrr Lol3rrr changed the title [WIP] Try to improve rust code readability Try to improve rust code readability Sep 23, 2024
@Lol3rrr
Copy link
Contributor Author

Lol3rrr commented Sep 23, 2024

Yeah I would be ready, in my previous comment I meant that there are potentially more things to do, but they would be in different PRs, sorry if that was not clear.

@LaihoE LaihoE merged commit 139b3ce into LaihoE:main Sep 23, 2024
6 checks passed
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