Skip to content
This repository has been archived by the owner on Sep 20, 2022. It is now read-only.

[HIVEMALL-142] Implement SingularizeUDF #110

Closed
wants to merge 4 commits into from

Conversation

takuti
Copy link
Member

@takuti takuti commented Aug 28, 2017

What changes were proposed in this pull request?

Implement singularize(string word) to obtain singular form of word.

The implementation referred the following third-party code:

What type of PR is it?

Feature

What is the Jira issue?

https://issues.apache.org/jira/browse/HIVEMALL-142

How was this patch tested?

unit test & manual test on EMR

How to use this feature?

as documented

Checklist

  • Did you apply source code formatter, i.e., mvn formatter:format, for your commit?

@@ -172,12 +172,17 @@ public static void clear(@Nonnull final StringBuilder buf) {

public static String concat(@Nonnull final List<String> list, @Nonnull final String sep) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@myui I guess you originally assumed this method behaves in a similar way to what org.apache.commons.lang3.StringUtils.join does. However, the original code appends a separator even at the end of result string as:

  • expected: concat(["a", "b", "c"], "-") => a-b-c
  • actual: concat(["a", "b", "c"], "-") => a-b-c-

So, I fixed the method in 796d388. Is this okay? If my assumption was incorrect, I revert the modification and introduce alternative method StringUtils.join().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

former is expected.

Copy link
Member Author

@takuti takuti Aug 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, so I revert and create join method here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

This behavior is similar to org.apache.commons.lang3.StringUtils.join
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 41.301% when pulling e230581 on takuti:singularize into 7205de1 on apache:master.

@asfgit asfgit closed this in 5e1d0d0 Sep 13, 2017
@myui
Copy link
Member

myui commented Sep 13, 2017

👍 Merged. Thanks!

@takuti takuti deleted the singularize branch September 14, 2017 05:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants