We help IT Professionals succeed at work.

Refactor a Java code

Rohit Bajaj
Rohit Bajaj asked
on
95 Views
Last Modified: 2017-05-01
Hi,
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(commands[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...

Thanks
Comment
Watch Question

CERTIFIED EXPERT
Commented:
This problem has been solved!
(Unlock this solution with a 7-day Free Trial)
UNLOCK SOLUTION

Author

Commented:
Hi,
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 ??
Thanks
CERTIFIED EXPERT

Commented:
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:

E.g.
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 :)

Doug
CERTIFIED EXPERT

Commented:
Only answer.