Refactor design smells #1986
Refactor design smells #1986
Conversation
|
Please why exactly are we doing this? What would we achieve from changing this particular code? |
Thanks for your review & feedback. By incorporating enum classes which have the following advantages. Enums are strongly typed, which means that the compiler will catch any errors at compile time rather than at runtime. This makes code more reliable and easier to debug. The other 2 benefits of using enum class is that 1) Enums can be extended to add new values. This makes our code more flexible and adaptable to changing requirements.2) Enums provide a more readable way to define a fixed set of values. This makes the code easier to understand and maintain. for the second case , I'm trying to incorporate pull up method refactoring to get rid of duplicate code , as it’s better to do some changes in a single place than have to search for all duplicates of the method in subclasses so overall objective in doing this changes are refactoring code as per the standard refactoring mechanism. In addition, as previously indicated in your remarks, I'm trying to address a few problems with ALTER functionalities. |
There was a problem hiding this comment.
I approve the refactoring of the String List Utils. This is actually very good and welcome.
I will not accept this Enums and the specific Keyword related stuff.
Please don't combine different issues and changes within on PR. PRs should be as distinct and encapsulated as possible please. For this one, focus on the String List Utils only. Once, this gets merged we can discuss other stuff.
There was a problem hiding this comment.
This is actually a good change, but please move those List Utils into a neutral Class which is not named anything like SELECT since Lists can be used anywhere.
There was a problem hiding this comment.
Definitely a No. You just scatter the code around without any benefit.
| @@ -41,83 +42,7 @@ public static String orderByToString(List<OrderByElement> orderByElements) { | |||
|
|
|||
| public static String orderByToString(boolean oracleSiblings, | |||
| List<OrderByElement> orderByElements) { | |||
| @@ -25,9 +25,5 @@ public MultiAndExpression(List<Expression> childlist) { | |||
| super(childlist); | |||
| } | |||
|
|
|||
| @Override | |||
| @@ -25,9 +25,5 @@ public MultiAndExpression(List<Expression> childlist) { | |||
| super(childlist); | |||
| } | |||
|
|
|||
| @Override | |||
| public String getStringExpression() { | |||
| @@ -19,9 +19,4 @@ public MultiOrExpression(List<Expression> childlist) { | |||
| super(childlist); | |||
| } | |||
|
|
|||
| @Override | |||
| @@ -78,4 +76,14 @@ public String toString() { | |||
| return sb.toString(); | |||
| } | |||
|
|
|||
| public String getStringExpression(){ | |||
Sure, I will create a separate PR for the String List Utils |
|
Closing as stale, please resubmit when work will have been finished. |
No description provided.