Avatar of Rohit Bajaj
Rohit Bajaj
Flag for India asked on

Refactor a Java code

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
Java

Avatar of undefined
Last Comment
dpearson

8/22/2022 - Mon
ASKER CERTIFIED SOLUTION
dpearson

Log in or sign up to see answer
Become an EE member today7-DAY FREE TRIAL
Members can start a 7-Day Free trial then enjoy unlimited access to the platform
Sign up - Free for 7 days
or
Learn why we charge membership fees
We get it - no one likes a content blocker. Take one extra minute and find out why we block content.
Not exactly the question you had in mind?
Sign up for an EE membership and get your own personalized solution. With an EE membership, you can ask unlimited troubleshooting, research, or opinion questions.
ask a question
Rohit Bajaj

ASKER
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
dpearson

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
dpearson

Only answer.
Experts Exchange has (a) saved my job multiple times, (b) saved me hours, days, and even weeks of work, and often (c) makes me look like a superhero! This place is MAGIC!
Walt Forbes