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