0
\$\begingroup\$

I'd like to refactor my extractSimplePromptName so it does the same but it's "prettier", hence less code. As far as looking at it, it seems like it could be refactored so it does the same thing, but it's just less code.

I'm guessing it has something to do with putting some stuff out of the first if sentence or perhaps just removing something that doesn't do anything.

@Test
public void makeSimpleName1() {
 String promptNameOrg = "20129142\1234円";
 String simplePromptName = extractSimplePromptName(promptNameOrg);
 Assert.assertEquals("1234", simplePromptName);
}
@Test
public void makeSimpleName2() {
 String promptNameOrg = "80808080\159円;20129142\1234円";
 String simplePromptName = extractSimplePromptName(promptNameOrg);
 Assert.assertEquals("1234", simplePromptName);
}
@Test
public void makeSimpleName3() {
 String promptNameOrg = "159;1234";
 String simplePromptName = extractSimplePromptName(promptNameOrg);
 Assert.assertEquals("1234", simplePromptName);
}
private String extractSimplePromptName(String promptNameOrg) {
 String simplePromptName = "";
 if (promptNameOrg != null) {
 if (promptNameOrg.contains(";") || promptNameOrg.contains("\\")) {
 List<String> splitPrompts = Arrays.asList(promptNameOrg.split(";"));
 String promptName = splitPrompts.get(splitPrompts.size() - 1);
 if (promptName.contains("\\")) {
 String[] split = promptName.split("\\\\");
 List<String> splitPromptName = Arrays.asList(split);
 simplePromptName = splitPromptName.get(splitPromptName.size() - 1);
 } else {
 simplePromptName = promptName;
 }
 } else {
 simplePromptName = promptNameOrg;
 }
 }
 return simplePromptName;
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Aug 27, 2014 at 9:55
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hey, welcome on CodeReview! If you would like to get more visibility on your question, I suggest you change your title to explain what your code does instead of asking a question (Unless your code is all about asking questions, then I don't know what to say about this.. :p ) \$\endgroup\$ Commented Sep 22, 2014 at 12:14

3 Answers 3

1
\$\begingroup\$

First thing I would do is reversing the ifs and returing the values directly instead of storing them in variables:

private String extractSimplePromptName(String promptNameOrg) {
 if (promptNameOrg == null) {
 return "";
 }
 if (!promptNameOrg.contains(";") && !promptNameOrg.contains("\\")) {
 return promptNameOrg;
 } 
 List<String> splitPrompts = Arrays.asList(promptNameOrg.split(";"));
 String promptName = splitPrompts.get(splitPrompts.size() - 1);
 if (promptName.contains("\\")) {
 String[] split = promptName.split("\\\\");
 List<String> splitPromptName = Arrays.asList(split);
 return splitPromptName.get(splitPromptName.size() - 1);
 } else {
 return promptName;
 }
}
answered Aug 27, 2014 at 10:16
\$\endgroup\$
2
  • \$\begingroup\$ This seems good, but you said first thing. Anything else to it? :) \$\endgroup\$ Commented Aug 27, 2014 at 10:22
  • \$\begingroup\$ @MikkelLarsen Nothing that comes to my mind at the moment. :) \$\endgroup\$ Commented Aug 27, 2014 at 10:27
2
\$\begingroup\$

If the parameter is null, we return the empty string.

If it doesn't contain either of our delimiters, we return the entire parameter.

Otherwise, we split on semicolon, taking the final term, and split the result on backslash, returning the final term. Note that splitting a string which doesn't contain the delimiter of interest just returns a list containing only the original string, the last element of which is of course the original string - so we can simplify that and say: just split and take the final term. Since we're doing that twice, extract into its own method:

private String extractSimplePromptName(String promptNameOrg) {
 String s = lastByDelimiter(promptNameOrg, ";");
 return lastByDelimiter(s, "\\\\");
}
private String lastByDelimiter(String s, String delimiter) {
 if (s == null || s.equals("")) {
 return "";
 }
 List<String> elements = Arrays.asList(s.split(delimiter));
 return elements.get(elements.size() - 1);
}
answered Aug 27, 2014 at 14:14
\$\endgroup\$
0
\$\begingroup\$

I think parameterized test is what you need.

@RunWith(Parameterized.class)
public class TestClass {
 private String input;
 private String expected;
 @Parameters
 public static Collection prepareData() {
 Object[][] object={
 {"20129142\1234円","1234"},
 {"80808080\159円;20129142\1234円","1234"},
 {"159;1234","1234"}
 };
 return Arrays.asList(object);
 }
 public TestClass(String input,String expected) {
 this.input=input;
 this.expected=expected;
 }
 @Test
 public void testMakeSimpleName() {
 String result= extractSimplePromptName(input);
 Assert.assertEquals(expected, result);
 }
}
answered Aug 27, 2014 at 10:33
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.