4

I have an java POJO class as below.

 public class EmployeeVo {
        private String firstName;
        private String middleName;
        private String lastName;
        private String suffix;
        private String addressline1;
        private String addressline2;
        private String city;
        private String state;
        private String zipCode;
        private String country;

            public String getFirstName() {
            return firstName;
        }
        public void setFirstName(String firstName) {
            this.firstName = firstName;
        }
        public String getMiddleName() {
            return middleName;
        }
        public void setMiddleName(String middleName) {
            this.middleName = middleName;
        }
        public String getLastName() {
            return lastName;
        }
        .// Setters and getters for other fields.
        .
        .

        }

Is there any way to check all the fields of an object at once for null , rather than checking each field for null?

While retrieving these values from EmployeeVo object

public static boolean isEmpty(String string){
    if(null==string || string.trim().length()==0){
        return true;
    }
    return false;
}

I have to manually null check each field before using it some where . As the above Object has more than 10 fields it would obviously increase the Cyclomatic Complexity for that method. Is there any optimal solution for it?

if(CommonUtil.isNotEmpty(employeeVO.getCity())){
            postalAddress.setCity(employeeVO.getCity());
        }
        if(CommonUtil.isNotEmpty(employeeVO.getCity())){
            postalAddress.setState(employeeVO.getState());
        }
        if(CommonUtil.isNotEmpty(employeeVO.getZipCode())){
            postalAddress.setPostalCode(employeeVO.getZipCode());
        }
        if(CommonUtil.isNotEmpty(employeeVO.getCountry())){
            postalAddress.setCountryCode(employeeVO.getCountry());
        }

Can we use

String country=employeeVO.getCountry();
postalAddress.setCountryCode( (country!=null) ? country: "");

Just to reduce the complexity? Is this according to standards?

Nayeem
  • 681
  • 15
  • 35

2 Answers2

5

it is not the responsibility of the caller to decide whether to set a field or not, it should be the method it self.

In your case this means the setters have to decide on the value of the parameter whether to set or not.

This moves the complexity of one method to the single individual methods and therefore decrease the complexity of the caller

Emerson Cod
  • 1,990
  • 3
  • 21
  • 39
  • You mean null check in Setter methods? – Nayeem Nov 10 '15 at 12:07
  • yes - the responsibility to decide whether a field gets a value set or not should be inside the class of this field and not outside, so the null check (and possible other checks) should be in the setter method. This also reduces the problem of every caller to make all checks – Emerson Cod Nov 10 '15 at 12:25
  • you have currently `if(CommonUtil.isNotEmpty(employeeVO.getCity())){ postalAddress.setState(employeeVO.getState()); }` what I suggest is simply `postalAddress.setState(employeeVO.getState());` and in the `setState` it is then `if(CommonUtil.isNotEmpty(state)) { this.state = state; }` – Emerson Cod Nov 10 '15 at 13:24
  • Thats actually present in a Jar file. So we cant change setState method. – Nayeem Nov 10 '15 at 13:33
  • Can we use String country=employeeVO.getCountry(); postalAddress.setCountryCode( (country!=null) ? country: ""); – Nayeem Nov 10 '15 at 13:41
2

This code does not reduce CC. You still have a branch when using ternary.

String country=employeeVO.getCountry();
postalAddress.setCountryCode( (country!=null) ? country: "");

I came up with a simple method for dealing with this.

ifNotBlank(String input, String output);

If the input is blank, then return the output. If the input is NOT blank then return the input. So when applied to your code it would look like this.

        postalAddress.setCity(ifNotBlank(employeeVO.getCity(), ""));
        postalAddress.setState(ifNotBlank(employeeVO.getState(), ""));
        postalAddress.setPostalCode(ifNotBlank(employeeVO.getZipCode(), ""));
        postalAddress.setCountryCode(ifNotBlank(employeeVO.getCountry(), ""));

The code for ifNotBlank would look like this.

import org.apache.commons.lang3.StringUtils;

public class ComplexityUtils{

    public static String ifNotBlank(String input, String output){
        return StringUtils.isBlank(input) ? output : input;
    }

}
Jose Martinez
  • 11,452
  • 7
  • 53
  • 68
  • Let me try this and scan with sonar! – Nayeem Nov 28 '15 at 07:09
  • 1
    Good idea in this situation, because according to Nayeem the postalAddress code cannot be changed. But if it could be changed, the change should be called internally in the postalAddress object. Each class should be responsible for keeping its data valid. This is pointed out by Emerson Cod – user2367418 Oct 30 '18 at 16:37