[GitHub] commons-text pull request #63: small-code-quality-improvements

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/commons-text/pull/63


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[TheRealHaui]

Github user TheRealHaui commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140548121
  
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,10 +614,7 @@ public static String swapCase(final String str) {
 for (int index = 0; index < strLen;) {
 final int oldCodepoint = str.codePointAt(index);
 final int newCodePoint;
-if (Character.isUpperCase(oldCodepoint)) {
-newCodePoint = Character.toLowerCase(oldCodepoint);
-whitespace = false;
-} else if (Character.isTitleCase(oldCodepoint)) {
+if (Character.isUpperCase(oldCodepoint) || 
Character.isTitleCase(oldCodepoint) ) {
--- End diff --

Removed white space.


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[chtompki]

Github user chtompki commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140545049
  
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,10 +614,7 @@ public static String swapCase(final String str) {
 for (int index = 0; index < strLen;) {
 final int oldCodepoint = str.codePointAt(index);
 final int newCodePoint;
-if (Character.isUpperCase(oldCodepoint)) {
-newCodePoint = Character.toLowerCase(oldCodepoint);
-whitespace = false;
-} else if (Character.isTitleCase(oldCodepoint)) {
+if (Character.isUpperCase(oldCodepoint) || 
Character.isTitleCase(oldCodepoint) ) {
--- End diff --

Checkstyle want's to see this as

```java
if (Character.isUpperCase(oldCodepoint) || 
Character.isTitleCase(oldCodepoint)) {
```


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[TheRealHaui]

Github user TheRealHaui commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140528491
  
--- Diff: src/main/java/org/apache/commons/text/FormattableUtils.java ---
@@ -46,6 +46,7 @@
  * This constructor is public to permit tools that require a 
JavaBean
  * instance to operate.
  */
+@SuppressWarnings("squid:S1118")
--- End diff --

Removed SuppressWarnings anootations completely.


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[PascalSchumacher]

Github user PascalSchumacher commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140526804
  
--- Diff: src/main/java/org/apache/commons/text/FormattableUtils.java ---
@@ -46,6 +46,7 @@
  * This constructor is public to permit tools that require a 
JavaBean
  * instance to operate.
  */
+@SuppressWarnings("squid:S1118")
--- End diff --

-1 to this

Eclipse was not displaying any warning before, but with this change it 
displays this warning: `Unsupported @SuppressWarnings("squid:S1118")`


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[TheRealHaui]

Github user TheRealHaui commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140526276
  
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,13 +615,11 @@ public static String swapCase(final String str) {
 for (int index = 0; index < strLen;) {
 final int oldCodepoint = str.codePointAt(index);
 final int newCodePoint;
-if (Character.isUpperCase(oldCodepoint)) {
+if (Character.isUpperCase(oldCodepoint) || 
Character.isTitleCase(oldCodepoint) ) {
 newCodePoint = Character.toLowerCase(oldCodepoint);
 whitespace = false;
-} else if (Character.isTitleCase(oldCodepoint)) {
-newCodePoint = Character.toLowerCase(oldCodepoint);
-whitespace = false;
-} else if (Character.isLowerCase(oldCodepoint)) {
+}
+else if (Character.isLowerCase(oldCodepoint)) {
--- End diff --

No, you're right.
My mistake.

Corrected and commited the correction.


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[TheRealHaui]

Github user TheRealHaui commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140525860
  
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,13 +615,11 @@ public static String swapCase(final String str) {
 for (int index = 0; index < strLen;) {
 final int oldCodepoint = str.codePointAt(index);
 final int newCodePoint;
-if (Character.isUpperCase(oldCodepoint)) {
+if (Character.isUpperCase(oldCodepoint) || 
Character.isTitleCase(oldCodepoint) ) {
--- End diff --

The corresponding lines as the whole method are already completely covered 
by existing Unit Tests.

![selection_050](https://user-images.githubusercontent.com/6312834/30752358-5a0d0094-9fbc-11e7-9ada-822cb344cefb.png)



---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[TheRealHaui]

Github user TheRealHaui commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140524746
  
--- Diff: src/main/java/org/apache/commons/text/FormattableUtils.java ---
@@ -46,6 +46,7 @@
  * This constructor is public to permit tools that require a 
JavaBean
  * instance to operate.
  */
+@SuppressWarnings("squid:S1118")
--- End diff --

Well, works with Sonaqube and Intellij IDEA.
Therefore I assume Eclipse and Netbeans recognizing it too.

I guess you want it to removed.
True?


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[kinow]

Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140401548
  
--- Diff: src/main/java/org/apache/commons/text/FormattableUtils.java ---
@@ -46,6 +46,7 @@
  * This constructor is public to permit tools that require a 
JavaBean
  * instance to operate.
  */
+@SuppressWarnings("squid:S1118")
--- End diff --

Is that a suppress warning for SonarQube? I am not sure if others are 
actively looking at the reports produced by SonarQube. I think this is not 
really necessary, if indeed just for SonarQube.


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[kinow]

Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140401739
  
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,13 +615,11 @@ public static String swapCase(final String str) {
 for (int index = 0; index < strLen;) {
 final int oldCodepoint = str.codePointAt(index);
 final int newCodePoint;
-if (Character.isUpperCase(oldCodepoint)) {
+if (Character.isUpperCase(oldCodepoint) || 
Character.isTitleCase(oldCodepoint) ) {
 newCodePoint = Character.toLowerCase(oldCodepoint);
 whitespace = false;
-} else if (Character.isTitleCase(oldCodepoint)) {
-newCodePoint = Character.toLowerCase(oldCodepoint);
-whitespace = false;
-} else if (Character.isLowerCase(oldCodepoint)) {
+}
+else if (Character.isLowerCase(oldCodepoint)) {
--- End diff --

Any reason for breaking the line here? Not sure if it's just GitHub UI 
getting confused, or if the bracket is not in the same line as the else if 
statement...


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[kinow]

Github user kinow commented on a diff in the pull request:

https://github.com/apache/commons-text/pull/63#discussion_r140401636
  
--- Diff: src/main/java/org/apache/commons/text/WordUtils.java ---
@@ -614,13 +615,11 @@ public static String swapCase(final String str) {
 for (int index = 0; index < strLen;) {
 final int oldCodepoint = str.codePointAt(index);
 final int newCodePoint;
-if (Character.isUpperCase(oldCodepoint)) {
+if (Character.isUpperCase(oldCodepoint) || 
Character.isTitleCase(oldCodepoint) ) {
--- End diff --

Nice catch. Any chance you could add a unit test for that too?


---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

[GitHub] commons-text pull request #63: small-code-quality-improvements

[TheRealHaui]

GitHub user TheRealHaui opened a pull request:

https://github.com/apache/commons-text/pull/63

small-code-quality-improvements

Made small code quality improvements.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TheRealHaui/commons-text 
small-code-quality-improvements

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-text/pull/63.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #63


commit d5c840c19792b6c0dee03e0779ac791c73f296cf
Author: Michael Hausegger 
Date:   2017-09-20T18:03:24Z

small-code-quality-improvements Made small code quality improvements.




---

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org