Refactor implementation smells #1982
Conversation
|
Greetings! Thanks for your effort and interest in JSQLParser. In my understanding building SQL code is one reasonable exception from various Java Code quality measurement because in this case long spaghetti code with lots of Although I will leave this open a bit for collecting opinions on this. |
|
Greetings!!
Thanks for considering my pull request and sharing your feedback.
I understand your concern about fragmenting into multiple methods,
potentially making it harder to trace the flow of SQL composition. The
intention behind my suggestion was to enhance maintainability and
scalability, especially for newcomers or when dealing with complex queries.
However, I completely agree that readability, especially in the context of
SQL parsing, is important.
Instead, I would humbly request you to have a look at this particular
commit:
ce72d72.
I have introduced an explaining variable to the method hashCode() making it
more readable and understandable. Please let me know your view on the same.
I can raise a separate pull request exclusively for this change.
I'm open to further discussions and ideas on how we can strike a balance
between readability and maintainability.
Thank you again for your feedback. I look forward to continuing to
contribute to the project.
Thanks and Regards
Nikita
…On Fri, Mar 29, 2024 at 10:24 PM manticore-projects < ***@***.***> wrote:
Greetings!
Thanks for your effort and interest in JSQLParser.
Unfortunately I am not a big fan of this change because it makes it harder
to understand the composition of the SQL since I have to jump now in
between various methods.
In my understanding building SQL code is one reasonable exception from
various Java Code quality measurement because in this case long spaghetti
code with lots of if/else conditions are actually better to read.
Although I will leave this open a bit for collecting opinions on this.
—
Reply to this email directly, view it on GitHub
<#1982 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AL2IIJJE5DY4TXL6CDYVA2LY2YH4XAVCNFSM6AAAAABFPD66M6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXHA3DANBRHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Apologies for being blunt: I do not see any point in this since the hash function is a kind of internal API and not used by anyone. You change is not useful or more readable but just takes more lines of code. Every IDE will generate the existing hash code for a good reason. I would be most suspicious when this presentation suddenly changed for no obvious reason.
We are riding the wrong horse here. If you really care about readability and maintainability then look at the various DDL statements like |
| @@ -366,14 +366,25 @@ public void visit(OverlapsCondition overlapsCondition) { | |||
| overlapsCondition.getRight().accept(this); | |||
| } | |||
|
|
|||
| @Override | |||
There was a problem hiding this comment.
I am not going to accept any of those changes because they don't improve readability of the code.
I'm working on refactoring CREATE and ALTER as well. |
|
Closing as stale, please resubmit when work will have been finished. |
No description provided.