Refactor a Java code

Rohit Bajaj
Rohit Bajaj used Ask the Experts™
I want to refactor the following code :
String sharerUserId = "", subCommand = "", repository = "";
 if (jsonNode != null) {
    String[] commands = jsonNode.asText().split(" ");
     sharerUserId = commands[0];
   StringBuilder subCommandBuffer = new StringBuilder();
      for (int i = 1; i < commands.length - 1; i++) {
        subCommandBuffer.append(" ");
 subCommand = subCommandBuffer.substring(0, subCommandBuffer.length() - 1);
       repository = commands[commands.length - 1];

Open in new window

In the above code i am extracting sharerUserId subCommand and repository from jsonNode.
This code was initially a part of a function. But i dont want to keep this code in that function as it destroys the SLAP principle that is single level of abstraction. To make the function cleaner i want to move this part out to a separate function. How do i do it ?
One way i think is to make a class with fields sharerUserId, subCommand and repository.
But I want to avoid this as this way i could come up with lots of unneccesary classes.. If i could pass these string to some other function which could be set from there ... but this is not possible as Strings will be passed by value.
Please suggest some Good approaches for the same...
The sharerUserId , subCommadn and repository is used later in the original function...

Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
You should add that extra class.

If you wish you can add it like this:

public class MainClass {
    public static class UserInfo {
          // sharerUserId , subCommand and repository

    public UserInfo parseJSON(JSONObject jsonNode) {
         return new UserInfo(sharerUserId , subCommand, repository) ;

Open in new window

so the additional class remains associated with the main class.  This is a reasonable structure if you just need the class to pass back results and don't plan to add methods to it or do anything else with the sub class.



I thought of one more approach... What if i take the code out into a separate function and pass a map like :
Map map = new HashMap<>();
into the function... and then set the three values sharerUserId, subCommand, and repository inside the map... This will also work...
What do you think of this approach compared to the above one which you suggested...
Is there any pros or cons in this ??
Yes passing in a Map would work.  However, I'd say it's not as good, because there's no guarantees that the values are set or that the names being used match:

Map map = new HashMap() ;
setMap(map) ;
String command = map.get("subcommand") ; // Fails - can you see why?

public void setMap(Map map) {
    map.put("subCommand", command) ;

Open in new window

A (small) class makes all of these things impossible.  The fields must be set (if you only set the values in the constructor) and if you ask for the wrong name, the compiler will tell you that it's wrong.

So a map would work, but not great here.  Lots of small classes are fine :)

Only answer.

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial