You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org> on 2023/11/22 04:58:50 UTC
[PR] Refactoring/push variable method technique [commons-dbutils]
sanjanarampurkottur01 opened a new pull request, #220:
URL: https://github.com/apache/commons-dbutils/pull/220
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402139176
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -53,6 +54,8 @@ public static ResultSet create(final ResultSetMetaData metaData, final Object[][
private Boolean wasNull = Boolean.FALSE;
+ private static final Set<String> METHOD_NAMES = Set.of("isLast", "hashCode", "toString", "equals");
Review Comment:
Make the Set immutable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org>.
sanjanarampurkottur01 commented on PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#issuecomment-1822571926
Hi Gary,
I am extremely sorry about that, I will change the formatting style and let
you know as soon as possible, once I push the changes.
Thank you.
Sincerely,
Sanjana Rampur Kottur
On Wed, 22 Nov 2023 at 07:09, Gary Gregory ***@***.***> wrote:
> @sanjanarampurkottur01 <https://github.com/sanjanarampurkottur01>
> Please don't change the formatting style, it makes PRs noisy and takes
> longer and is harder to review.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-dbutils/pull/220#issuecomment-1822565139>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BDTL2564LNKWE5HMZCKELHDYFXMOLAVCNFSM6AAAAAA7VT4TEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRSGU3DKMJTHE>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#issuecomment-1822565139
@sanjanarampurkottur01
Please don't change the formatting style, it makes PRs noisy and takes longer and is harder to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402139176
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -53,6 +54,8 @@ public static ResultSet create(final ResultSetMetaData metaData, final Object[][
private Boolean wasNull = Boolean.FALSE;
+ private static final Set<String> METHOD_NAMES = Set.of("isLast", "hashCode", "toString", "equals");
Review Comment:
Make the Set immutable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org>.
sanjanarampurkottur01 commented on PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#issuecomment-1822576411
> @sanjanarampurkottur01 Please don't change the formatting style, it makes PRs noisy and takes longer and is harder to review.
@garydgregory , I will fix that andl et you know. I am extremely sorry for the inconvenience caused.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org>.
sanjanarampurkottur01 commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402081588
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -265,64 +266,86 @@ protected String getString(final int columnIndex) throws SQLException {
}
@Override
- public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
+ public Object invoke(final Object proxy, final Method method, final Object[] args)
+ throws Throwable {
final String methodName = method.getName();
-
if (methodName.equals("getMetaData")) {
- return this.getMetaData();
-
+ return handleGetMetaData();
}
if (methodName.equals("next")) {
- return this.next();
-
+ return handleNext();
}
- if (methodName.equals("previous") || methodName.equals("close")) {
-
- } else if (methodName.equals("getBoolean")) {
- return this.getBoolean(columnIndex(args));
-
- } else if (methodName.equals("getByte")) {
- return this.getByte(columnIndex(args));
-
- } else if (methodName.equals("getDouble")) {
- return this.getDouble(columnIndex(args));
-
- } else if (methodName.equals("getFloat")) {
- return this.getFloat(columnIndex(args));
-
- } else if (methodName.equals("getInt")) {
- return this.getInt(columnIndex(args));
-
- } else if (methodName.equals("getLong")) {
- return this.getLong(columnIndex(args));
-
- } else if (methodName.equals("getObject")) {
- return this.getObject(columnIndex(args));
-
- } else if (methodName.equals("getShort")) {
- return this.getShort(columnIndex(args));
+ if (methodName.equals("previous")) {
+ // Handle previous method
+ } else if (methodName.equals("close")) {
+ // Handle close method
+ } else if (isColumnMethod(methodName)) {
+ return handleColumnMethod(methodName, args);
+ } else if (isNonColumnMethod(methodName)) {
+ return handleNonColumnMethod(methodName, proxy, args);
+ }
+ throw new UnsupportedOperationException("Unsupported method: " + methodName);
+ }
- } else if (methodName.equals("getString")) {
- return this.getString(columnIndex(args));
+ // Define methods for handling specific cases
+ private Object handleGetMetaData() throws SQLException {
Review Comment:
Resolved it. Thank you :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402050005
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -21,10 +21,7 @@
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
Review Comment:
and please rebase on master ;-) TY!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org>.
sanjanarampurkottur01 commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402062601
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -21,10 +21,7 @@
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
Review Comment:
@garydgregory ,
Fixed it :)
Kindly, review the same. Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org>.
sanjanarampurkottur01 commented on PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#issuecomment-1822600824
Hi Gary,
I have made the necessary changes to
https://github.com/apache/commons-dbutils/pull/220, I kindly request you to
review and approve the same.
My sincere apologies for the same.
Thank you.
Sincerely,
Sanjana Rampur Kottur
On Wed, 22 Nov 2023 at 07:13, Sanjana RK ***@***.***>
wrote:
> Hi Gary,
>
> I am extremely sorry about that, I will change the formatting style and
> let you know as soon as possible, once I push the changes.
>
> Thank you.
>
> Sincerely,
> Sanjana Rampur Kottur
>
> On Wed, 22 Nov 2023 at 07:09, Gary Gregory ***@***.***>
> wrote:
>
>> @sanjanarampurkottur01 <https://github.com/sanjanarampurkottur01>
>> Please don't change the formatting style, it makes PRs noisy and takes
>> longer and is harder to review.
>>
>> —
>> Reply to this email directly, view it on GitHub
>> <https://github.com/apache/commons-dbutils/pull/220#issuecomment-1822565139>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/BDTL2564LNKWE5HMZCKELHDYFXMOLAVCNFSM6AAAAAA7VT4TEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRSGU3DKMJTHE>
>> .
>> You are receiving this because you were mentioned.Message ID:
>> ***@***.***>
>>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402070369
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -265,64 +266,86 @@ protected String getString(final int columnIndex) throws SQLException {
}
@Override
- public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
+ public Object invoke(final Object proxy, final Method method, final Object[] args)
+ throws Throwable {
final String methodName = method.getName();
-
if (methodName.equals("getMetaData")) {
- return this.getMetaData();
-
+ return handleGetMetaData();
}
if (methodName.equals("next")) {
- return this.next();
-
+ return handleNext();
}
- if (methodName.equals("previous") || methodName.equals("close")) {
-
- } else if (methodName.equals("getBoolean")) {
- return this.getBoolean(columnIndex(args));
-
- } else if (methodName.equals("getByte")) {
- return this.getByte(columnIndex(args));
-
- } else if (methodName.equals("getDouble")) {
- return this.getDouble(columnIndex(args));
-
- } else if (methodName.equals("getFloat")) {
- return this.getFloat(columnIndex(args));
-
- } else if (methodName.equals("getInt")) {
- return this.getInt(columnIndex(args));
-
- } else if (methodName.equals("getLong")) {
- return this.getLong(columnIndex(args));
-
- } else if (methodName.equals("getObject")) {
- return this.getObject(columnIndex(args));
-
- } else if (methodName.equals("getShort")) {
- return this.getShort(columnIndex(args));
+ if (methodName.equals("previous")) {
+ // Handle previous method
+ } else if (methodName.equals("close")) {
+ // Handle close method
+ } else if (isColumnMethod(methodName)) {
+ return handleColumnMethod(methodName, args);
+ } else if (isNonColumnMethod(methodName)) {
+ return handleNonColumnMethod(methodName, proxy, args);
+ }
+ throw new UnsupportedOperationException("Unsupported method: " + methodName);
+ }
- } else if (methodName.equals("getString")) {
- return this.getString(columnIndex(args));
+ // Define methods for handling specific cases
+ private Object handleGetMetaData() throws SQLException {
Review Comment:
This is superfluous IMO; just leave the implementation inlined as it was.
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -265,64 +266,86 @@ protected String getString(final int columnIndex) throws SQLException {
}
@Override
- public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
+ public Object invoke(final Object proxy, final Method method, final Object[] args)
+ throws Throwable {
final String methodName = method.getName();
-
if (methodName.equals("getMetaData")) {
- return this.getMetaData();
-
+ return handleGetMetaData();
}
if (methodName.equals("next")) {
- return this.next();
-
+ return handleNext();
}
- if (methodName.equals("previous") || methodName.equals("close")) {
-
- } else if (methodName.equals("getBoolean")) {
- return this.getBoolean(columnIndex(args));
-
- } else if (methodName.equals("getByte")) {
- return this.getByte(columnIndex(args));
-
- } else if (methodName.equals("getDouble")) {
- return this.getDouble(columnIndex(args));
-
- } else if (methodName.equals("getFloat")) {
- return this.getFloat(columnIndex(args));
-
- } else if (methodName.equals("getInt")) {
- return this.getInt(columnIndex(args));
-
- } else if (methodName.equals("getLong")) {
- return this.getLong(columnIndex(args));
-
- } else if (methodName.equals("getObject")) {
- return this.getObject(columnIndex(args));
-
- } else if (methodName.equals("getShort")) {
- return this.getShort(columnIndex(args));
+ if (methodName.equals("previous")) {
+ // Handle previous method
+ } else if (methodName.equals("close")) {
+ // Handle close method
+ } else if (isColumnMethod(methodName)) {
+ return handleColumnMethod(methodName, args);
+ } else if (isNonColumnMethod(methodName)) {
+ return handleNonColumnMethod(methodName, proxy, args);
+ }
+ throw new UnsupportedOperationException("Unsupported method: " + methodName);
+ }
- } else if (methodName.equals("getString")) {
- return this.getString(columnIndex(args));
+ // Define methods for handling specific cases
+ private Object handleGetMetaData() throws SQLException {
+ return this.getMetaData();
+ }
- } else if (methodName.equals("wasNull")) {
- return this.wasNull();
+ private Object handleNext() throws SQLException {
Review Comment:
This is superfluous IMO; just leave the implementation inlined as it was.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402094200
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -265,64 +266,77 @@ protected String getString(final int columnIndex) throws SQLException {
}
@Override
- public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
+ public Object invoke(final Object proxy, final Method method, final Object[] args)
+ throws Throwable {
final String methodName = method.getName();
-
if (methodName.equals("getMetaData")) {
return this.getMetaData();
-
}
if (methodName.equals("next")) {
return this.next();
-
}
- if (methodName.equals("previous") || methodName.equals("close")) {
-
- } else if (methodName.equals("getBoolean")) {
- return this.getBoolean(columnIndex(args));
-
- } else if (methodName.equals("getByte")) {
- return this.getByte(columnIndex(args));
-
- } else if (methodName.equals("getDouble")) {
- return this.getDouble(columnIndex(args));
-
- } else if (methodName.equals("getFloat")) {
- return this.getFloat(columnIndex(args));
-
- } else if (methodName.equals("getInt")) {
- return this.getInt(columnIndex(args));
-
- } else if (methodName.equals("getLong")) {
- return this.getLong(columnIndex(args));
-
- } else if (methodName.equals("getObject")) {
- return this.getObject(columnIndex(args));
-
- } else if (methodName.equals("getShort")) {
- return this.getShort(columnIndex(args));
-
- } else if (methodName.equals("getString")) {
- return this.getString(columnIndex(args));
-
- } else if (methodName.equals("wasNull")) {
- return this.wasNull();
+ if (methodName.equals("previous")) {
+ // Handle previous method
+ } else if (methodName.equals("close")) {
+ // Handle close method
+ } else if (isColumnMethod(methodName)) {
+ return handleColumnMethod(methodName, args);
+ } else if (isNonColumnMethod(methodName)) {
+ return handleNonColumnMethod(methodName, proxy, args);
+ }
+ throw new UnsupportedOperationException("Unsupported method: " + methodName);
+ }
- } else if (methodName.equals("isLast")) {
- return this.isLast();
+ private boolean isColumnMethod(String methodName) {
+ return methodName.startsWith("get") || methodName.equals("wasNull");
+ }
- } else if (methodName.equals("hashCode")) {
- return Integer.valueOf(System.identityHashCode(proxy));
+ private Object handleColumnMethod(String methodName, final Object[] args) throws SQLException {
+ switch (methodName) {
+ case "getBoolean":
+ return this.getBoolean(columnIndex(args));
+ case "getByte":
+ return this.getByte(columnIndex(args));
+ case "getDouble":
+ return this.getDouble(columnIndex(args));
+ case "getFloat":
+ return this.getFloat(columnIndex(args));
+ case "getInt":
+ return this.getInt(columnIndex(args));
+ case "getLong":
+ return this.getLong(columnIndex(args));
+ case "getObject":
+ return this.getObject(columnIndex(args));
+ case "getShort":
+ return this.getShort(columnIndex(args));
+ case "getString":
+ return this.getString(columnIndex(args));
+ case "wasNull":
+ return this.wasNull();
+ default:
+ throw new UnsupportedOperationException("Unsupported column method: " + methodName);
+ }
+ }
- } else if (methodName.equals("toString")) {
- return "MockResultSet " + System.identityHashCode(proxy);
+ private boolean isNonColumnMethod(String methodName) {
+ Set<String> methodNames = Set.of("isLast", "hashCode", "toString", "equals");
Review Comment:
This Set does not need to be created on each invocation, just make it a constant and immutable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402045445
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -21,10 +21,7 @@
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
Review Comment:
Please Don't change to star (`*`) imports.
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -21,10 +21,7 @@
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
Review Comment:
Please don't change to star (`*`) imports.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402045445
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -21,10 +21,7 @@
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
Review Comment:
Don't change to start imports.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org>.
sanjanarampurkottur01 commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402115817
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -265,64 +266,77 @@ protected String getString(final int columnIndex) throws SQLException {
}
@Override
- public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
+ public Object invoke(final Object proxy, final Method method, final Object[] args)
+ throws Throwable {
final String methodName = method.getName();
-
if (methodName.equals("getMetaData")) {
return this.getMetaData();
-
}
if (methodName.equals("next")) {
return this.next();
-
}
- if (methodName.equals("previous") || methodName.equals("close")) {
-
- } else if (methodName.equals("getBoolean")) {
- return this.getBoolean(columnIndex(args));
-
- } else if (methodName.equals("getByte")) {
- return this.getByte(columnIndex(args));
-
- } else if (methodName.equals("getDouble")) {
- return this.getDouble(columnIndex(args));
-
- } else if (methodName.equals("getFloat")) {
- return this.getFloat(columnIndex(args));
-
- } else if (methodName.equals("getInt")) {
- return this.getInt(columnIndex(args));
-
- } else if (methodName.equals("getLong")) {
- return this.getLong(columnIndex(args));
-
- } else if (methodName.equals("getObject")) {
- return this.getObject(columnIndex(args));
-
- } else if (methodName.equals("getShort")) {
- return this.getShort(columnIndex(args));
-
- } else if (methodName.equals("getString")) {
- return this.getString(columnIndex(args));
-
- } else if (methodName.equals("wasNull")) {
- return this.wasNull();
+ if (methodName.equals("previous")) {
+ // Handle previous method
+ } else if (methodName.equals("close")) {
+ // Handle close method
+ } else if (isColumnMethod(methodName)) {
+ return handleColumnMethod(methodName, args);
+ } else if (isNonColumnMethod(methodName)) {
+ return handleNonColumnMethod(methodName, proxy, args);
+ }
+ throw new UnsupportedOperationException("Unsupported method: " + methodName);
+ }
- } else if (methodName.equals("isLast")) {
- return this.isLast();
+ private boolean isColumnMethod(String methodName) {
+ return methodName.startsWith("get") || methodName.equals("wasNull");
+ }
- } else if (methodName.equals("hashCode")) {
- return Integer.valueOf(System.identityHashCode(proxy));
+ private Object handleColumnMethod(String methodName, final Object[] args) throws SQLException {
+ switch (methodName) {
+ case "getBoolean":
+ return this.getBoolean(columnIndex(args));
+ case "getByte":
+ return this.getByte(columnIndex(args));
+ case "getDouble":
+ return this.getDouble(columnIndex(args));
+ case "getFloat":
+ return this.getFloat(columnIndex(args));
+ case "getInt":
+ return this.getInt(columnIndex(args));
+ case "getLong":
+ return this.getLong(columnIndex(args));
+ case "getObject":
+ return this.getObject(columnIndex(args));
+ case "getShort":
+ return this.getShort(columnIndex(args));
+ case "getString":
+ return this.getString(columnIndex(args));
+ case "wasNull":
+ return this.wasNull();
+ default:
+ throw new UnsupportedOperationException("Unsupported column method: " + methodName);
+ }
+ }
- } else if (methodName.equals("toString")) {
- return "MockResultSet " + System.identityHashCode(proxy);
+ private boolean isNonColumnMethod(String methodName) {
+ Set<String> methodNames = Set.of("isLast", "hashCode", "toString", "equals");
Review Comment:
Modified this as well. Thanks for noticing it. Static field will be a better choice. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactoring/push variable method technique [commons-dbutils]
Posted by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org>.
sanjanarampurkottur01 commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402049550
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -21,10 +21,7 @@
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
Review Comment:
Sure @garydgregory ,
I'm currently addressing the issue, and I appreciate your prompt review.
Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "sanjanarampurkottur01 (via GitHub)" <gi...@apache.org>.
sanjanarampurkottur01 commented on code in PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#discussion_r1402082036
##########
src/test/java/org/apache/commons/dbutils/MockResultSet.java:
##########
@@ -265,64 +266,86 @@ protected String getString(final int columnIndex) throws SQLException {
}
@Override
- public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable {
+ public Object invoke(final Object proxy, final Method method, final Object[] args)
+ throws Throwable {
final String methodName = method.getName();
-
if (methodName.equals("getMetaData")) {
- return this.getMetaData();
-
+ return handleGetMetaData();
}
if (methodName.equals("next")) {
- return this.next();
-
+ return handleNext();
}
- if (methodName.equals("previous") || methodName.equals("close")) {
-
- } else if (methodName.equals("getBoolean")) {
- return this.getBoolean(columnIndex(args));
-
- } else if (methodName.equals("getByte")) {
- return this.getByte(columnIndex(args));
-
- } else if (methodName.equals("getDouble")) {
- return this.getDouble(columnIndex(args));
-
- } else if (methodName.equals("getFloat")) {
- return this.getFloat(columnIndex(args));
-
- } else if (methodName.equals("getInt")) {
- return this.getInt(columnIndex(args));
-
- } else if (methodName.equals("getLong")) {
- return this.getLong(columnIndex(args));
-
- } else if (methodName.equals("getObject")) {
- return this.getObject(columnIndex(args));
-
- } else if (methodName.equals("getShort")) {
- return this.getShort(columnIndex(args));
+ if (methodName.equals("previous")) {
+ // Handle previous method
+ } else if (methodName.equals("close")) {
+ // Handle close method
+ } else if (isColumnMethod(methodName)) {
+ return handleColumnMethod(methodName, args);
+ } else if (isNonColumnMethod(methodName)) {
+ return handleNonColumnMethod(methodName, proxy, args);
+ }
+ throw new UnsupportedOperationException("Unsupported method: " + methodName);
+ }
- } else if (methodName.equals("getString")) {
- return this.getString(columnIndex(args));
+ // Define methods for handling specific cases
+ private Object handleGetMetaData() throws SQLException {
+ return this.getMetaData();
+ }
- } else if (methodName.equals("wasNull")) {
- return this.wasNull();
+ private Object handleNext() throws SQLException {
Review Comment:
Resolved this as well!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Switch on String instead of cascading if-else [commons-dbutils]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #220:
URL: https://github.com/apache/commons-dbutils/pull/220#issuecomment-1822863551
## [Codecov](https://app.codecov.io/gh/apache/commons-dbutils/pull/220?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`b003601`)](https://app.codecov.io/gh/apache/commons-dbutils/commit/b003601f5197acf4429c003281c95b2a24f0c04f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 62.63% compared to head [(`20d8fc8`)](https://app.codecov.io/gh/apache/commons-dbutils/pull/220?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 62.63%.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #220 +/- ##
=========================================
Coverage 62.63% 62.63%
Complexity 411 411
=========================================
Files 39 39
Lines 1381 1381
Branches 104 104
=========================================
Hits 865 865
Misses 467 467
Partials 49 49
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/commons-dbutils/pull/220?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org