I've been shown the following Java code as part of a code review and it's quite frankly hideous. However, I'm at a loss as to what the best way to re-write it is.
We have a list that's always in the same order (0-5) and each index corresponds to a different property that is required to be set. Using groovy it would be easy as it would be myObject."setLine${i}" or similar but how can we achieve the same in plain Java?
SomeObject myObject = new SomeObject();
List<String> lines = new ArrayList<>(){{
add("Line 1");
add("Line 2");
add("Line 3");
add("Line 4");
add("Line 5");
add("Line 6");
}};
for(int i=0; i< lines.size(); i++){
if(!StringUtils.isBlank(lines.get(i))){
String line = lines.get(i);
// line1
if(i==0){
myObject.setLine1(line);
}
// line2
if(i==1){
myObject.setLine2(line);
}
// line3
if(i==2){
myObject.setLine3(line);
}
// line4
if(i==3){
myObject.setLine4(line);
}
// line5
if(i==4){
myObject.setLine5(line);
}
//line6
if(i==5){
myObject.setLine6(line);
}
}
}
I know that for:each can provide us with an index but it's the "dynamic" method calling that I'm querying. Any suggestion welcome - I'm considering that the MyObject class should maybe have a method to take in a list of address strings so that it doesn't couple quite so tightly by exposing the number of lines to the caller (ie if we add/remove a "setLineX" method we don't want to have to update the caller).
The first issue is in myObject. Having set methods for each line is, as you can see, tedious. How about something like this?
class MyObject{
private Map<Integer,String> lines = new HashMap<>();
public void setLine(int lineNumber, String line){
lines.put(lineNumber,line);
}
public String getLine(int lineNumber){
lines.get(lineNumber);
}
}
Now you can simply do
for(int i=0;i<lineListSize;i++){
myOjbect.setLine(i,lineList.get(i));
}
EDIT: As Fabian just pointed out, you could do the same with a List.
class MyObject{
private List<String> lines = new ArrayList<>();
public void setLine(int lineNumber, String line){
lines.add(lineNumber,line);
}
public String getLine(int lineNumber){
lines.get(lineNumber);
}
}
EDIT2: I can never leave well-enough-alone. You can make the transfer even cleaner with
class MyObject{
...
// Collection, getter and setter omitted
...
public void addAllLines(List<String> lines){
// implementation varies depending on collection,
// but List would simply be "this.lines.addAll(lines);"
}
public List<String> getAllLines(){
// Again, depends on the type of Collection used by
// this instance.
// Probably want to return a copy if thread safety is an issue.
}
}
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With