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.

Sign up for GitHub

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

Jump to bottom

[Feature Request] Unnecessary getters on Config sturct #401

Closed
Antosser opened this issue Nov 15, 2023 · 4 comments
Closed

[Feature Request] Unnecessary getters on Config sturct #401

Antosser opened this issue Nov 15, 2023 · 4 comments
Assignees
@Antosser
Labels
enhancement Existing behavior/feature improvement good first issue Good for newcomers

Comments

@Antosser
Copy link
Contributor

Antosser commented Nov 15, 2023 •
edited
Loading

http-server/src/config/mod.rs

Line 4 in 7eabb9b

pub mod file;

I really don't understand why you'd want getters for these fields. It seems unnecessary because you could simply make the fields public.

If the goal was to make the fields immutable, you could just instantiate the struct without the mut keyword
@EstebanBorai
Copy link
Member

I agree with you!

Given that this was my first Rust project, back in the day I had other POV on software design!

Good observation!

PR welcome!

@Antosser
Copy link
Contributor Author

I'd wait for #374 before doing this one because I don't really want to introduce any merge conflicts

@EstebanBorai
Copy link
Member

I'd wait for #374 before doing this one because I don't really want to introduce any merge conflicts

Sgtm!

@Antosser
Copy link
Contributor Author

I'll get on with this one since #374 is done

@Antosser Antosser mentioned this issue Feb 28, 2024
Merged
@Antosser Antosser self-assigned this Feb 28, 2024
@EstebanBorai EstebanBorai added enhancement Existing behavior/feature improvement good first issue Good for newcomers labels Feb 28, 2024
github-merge-queue bot pushed a commit that referenced this issue Feb 28, 2024
@Antosser
feat: Replace unnecessary getters on Config struct (#423)
a9fd962 
Implemented #401 

All I did was make the fields public and delete the getters. I decided
to try to remove the unnecessary `clone`s in a separate PR after this
one gets merged for transparency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Antosser Antosser

Labels
enhancement Existing behavior/feature improvement good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@EstebanBorai @Antosser