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;
}
-
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\$IEatBagels– IEatBagels2014年09月22日 12:14:17 +00:00Commented Sep 22, 2014 at 12:14
3 Answers 3
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;
}
}
-
\$\begingroup\$ This seems good, but you said first thing. Anything else to it? :) \$\endgroup\$Mikkel Larsen– Mikkel Larsen2014年08月27日 10:22:24 +00:00Commented Aug 27, 2014 at 10:22
-
\$\begingroup\$ @MikkelLarsen Nothing that comes to my mind at the moment. :) \$\endgroup\$pmichna– pmichna2014年08月27日 10:27:23 +00:00Commented Aug 27, 2014 at 10:27
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);
}
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);
}
}