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

Jug refactor: Separate information printing logic and its invocation #115

Closed

Conversation

divinenickname
Copy link
Contributor

Nowadays, the Jug class is overcoded and contains too much complexity. This class violates the S in the SOLID principles. I see the need to fix it.

In this PR, I separated usage information formation and printing. I think it’s better to place it in a different package, e.g., jug, but this is a discussion point. Also, I added an extra test case and checked the whole information output to prevent inconsistent data.

@cowtowncoder
Copy link
Owner

Hmmh. I wonder if you are solving an actual problem, instead of theoretical/philosophical one?
I don't argue against codebase having issues, but it is rather small a library and it's sometimes easy to go bike-shedding for questionable benefits (and hence I do actually question "over-engineered" part come to think of it).
I am not a big fan of refactoring for sake of refactoring, and given that future development needs are likely limited I wonder.

So I am not sure I agree with the " I see the need to fix it." part.

Let me think a bit about this; maybe I can take pieces I think do matter (like javadoc generation warning)

cowtowncoder added a commit that referenced this pull request Jun 30, 2024
@cowtowncoder
Copy link
Owner

Ok, so, first of all: thank you for contributing this pr @divinenickname . Updates to README, pom.xml make sense. I merged those separately.

But I do not see value in refactoring itself, and in particular not for unit test checking whether usage information would be printed exactly as defined in source, duplicated (somewhat violating DRY principle: and if usage message is to be changed now having to modify in 2 places).

So I will not be proceeding with other parts of this PR.

@divinenickname
Copy link
Contributor Author

Hmmh. I wonder if you are solving an actual problem, instead of theoretical/philosophical one? I don't argue against codebase having issues, but it is rather small a library and it's sometimes easy to go bike-shedding for questionable benefits (and hence I do actually question "over-engineered" part come to think of it). I am not a big fan of refactoring for sake of refactoring, and given that future development needs are likely limited I wonder.

So I am not sure I agree with the " I see the need to fix it." part.

Let me think a bit about this; maybe I can take pieces I think do matter (like javadoc generation warning)

I made this small pull request to suggest refactoring the JUG class. I want to move and encapsulate CLI interactions away from the core UUID logic. My main point is to avoid mixing core logic with CLI interactions and make the CLI more maintainable than it is now.

upd. I saw your comment. Your point is absolutely valid.

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