Splitting strings in Java

Question

I wrote a function which takes in input a list of strings that is splitted by comma. Then each generated string is trimmed.

Example:

INPUT:    [[Cat, Dog, Snake]]
OUTPUT:   Cat Dog Snake

In particular, I'm interested in removing parenthesis and commas. This is the code I wrote, but I think it is awful and not so efficient because I use so many String and LinkedList variables.

private List<String> splitListOfString(LinkedList<String> list)
{
String mylist = list.toString(); //is made by parenthesis, I do not want
mylist = mylist.substring(2);  //delete the first 2
mylist = mylist.substring(0, mylist.length() - 2); //delete the last 2
List<String> thelist = Arrays.asList(mylist.split(","));
List<String> thelist2 = new LinkedList<String>();
for (String string : thelist) {
    thelist2.add(string.trim());
}
    return thelist2;
}

The line of code involving the trim function is compulsory to avoid something like " string", that is undesirable. But I do not think that this is the best way to write this code. How would you improve this?

Answer 1

I'm not sure how you got the original list-within-a-list input, but have you looked into Java Streams? Here's a tutorial that may help.

It looks like you're taking in a LinkedList, turning it into a string to manually delete the [[ and ]], splitting on comma, then adding each item to a new list.

Why not do something like the following:

private static List<String> splitListOfString(LinkedList<String> list) { 
    List<String> myList = new LinkedList<>();
    list.stream().forEach(item -> myList.add(item));
    return myList;
}

In the above code, you're leaving the elements in the list (instead of converting to string), retrieving each item, then adding to your output list. It may need to be modified slightly to match your input.

Answer 2

---

Formatting

Your overall code formatting leaves to be desired. Please use consistent blocks and whitespace.

It helps hugely in readability. Most IDEs will do it automatically for you whenever the file is saved. Pay attention to how I used whitespaces to convey meaning in my last code snippet.

---

Naming

Here are your variables:

list    
myList
theList
theList2

Not terribly clear, is it?

Some of them are Lists, some are Strings. This is further confusing. Also what does each one do? It is really not obvious because their naming does not tell us.

You should call them with meaningful names like:

list      --> inputList
myList    --> comaSeparatedElement
theList   --> workingList
theList2  --> resultList

Naming things correctly is hugely important:

• For you: to keep your algo and your concept under control. Naming gives control.
• For your readers: to read your mind, because you're not there to tell them, and you've forgotten about the code and moved on with your life.

---

Try to review your requirements

Apparently the animals are in a String, that String is formatted as if it was a animalList.toString(), and the String is also, itself, in a List.

Using a String is a very bad choice to communicate a list of animals. Can you change it? If so, please do so. Also, have you thought about what happens if the input contains two String like :

["[Cat, Dog]", "[Snake, Scorpion]"]

Then do all elements need to go into the same output List?

---

Don't over-constrain your inputs

What if I had an ArrayList I wanted to submit? I can't use your function, it requires a LinkedList. What if I had a List<String>,and didn't even know its implementation? Sure, I can copy it all to a LinkedList, but its such a waste...

You're not even using the knowledge of your specific List type! LinkedList has getFirst(), getLast(), a spliterator etc. But you're not using that. You're only toStringing it. So you can open up your method, remove the constraint, and have this signature:

private List<String> splitListOfString(List<String> list);

---

Keep the data in its original format

The first thing you did is list.toString(). This is messing up the input, which was a nice, practical, List. What if the author of LinkedList decided to change the format of its toString() method? Or if the LinkedList you got was actually subclassed by someone who modified the toString() method? Keep the data in its original format.

Browse the content using built-in foreach iteration:

for(String comaSeparatedElement: inputList){
    // Dosomething with that comaSeparatedElement 
}

---

Substring in one shot

You used substring, which is nice and efficient.

You should have read about the other substring method, the one with start and end arguments:

comaSeparatedElement = comaSeparatedElement.substring(1, comaSeparatedElement.length() - 1); //delete the 1st and last characters(those are the '[' and the ']'

---

Unleash Java 8

As @ErikaBurdon noted, the remaining 4 lines of code can be advantageously replaced by some concise, and powerful Stream calls.

I can propose a full-strem solution, even though here it looks a bit contrived:

• Streaming the input List
• Using flatMap to collect in a single Stream all the pieces of each element, because each coma-separated element is:
  • substringed to remove the leading '[' and trailing ']'
  • split at every ',' to make an array of String pieces
  • That array is Streamed (using the Arrays utility class)
  • Each element of the sub-stream is trimmed using map
• Collecting the trimmed Strings in a List

Here you go:

private List<String> splitListOfString(List<String> list) {
    return inputList.stream()                // Stream the input
             .flatMap(                       // collect al pieces in a single list
                  comaSeparatedList -> Arrays.stream(comaSeparatedList.substring(1, comaSeparatedList.length() - 1)
                               .split(","))) // Split each string in pieces here
             .map(String::trim)              // Trim each piece
             .collect(Collectors.toList());  // Collect in a single list
}