1

I have a DTO with a few variables and I need to check if my DTO is empty.

I looked into older questions and found this one which is quite interesting Check object empty

But I have the feeling that my solution can be improved as it´s something quite raw.

Here is my code.

 public boolean isEmpty() {
            return StringUtils.isBlank(nombre) && StringUtils.isBlank(apellidos1) && StringUtils.isBlank(apellidos2) && StringUtils.isBlank(NIF) 
                    && StringUtils.isBlank(nFederado) && StringUtils.isBlank(fecha) && StringUtils.isBlank(poliza) && StringUtils.isBlank(estado) 
                    && StringUtils.isBlank(numeroLesion);
    }

I use this function in the action of an API to check if it´s completely empty. here is my action:

 @Override
    public Map execute(Map params) throws Exception {
        log.error(" En el action consultar federados action (en el punto que queremos)  ");
        ConsultarFederado consultaFederado = new ConsultarFederado();
        Map result = new HashMap<>();
        Map requestParams = (Map) params.get("request_params");
            consultaFederado.setNombre(setValue("nombre", requestParams));
            consultaFederado.setApellidos1(setValue("apellidos1", requestParams));
            consultaFederado.setApellidos2(setValue("apellidos2", requestParams));
            consultaFederado.setNIF(setValue("nif", requestParams));
            consultaFederado.setFecha(setValue("fecha", requestParams));
            consultaFederado.setPoliza(setValue("poliza", requestParams));
            consultaFederado.setnFederado(setValue("nFederado", requestParams));
            consultaFederado.setEstado(setValue("estado", requestParams));
            consultaFederado.setNumeroLesion(setValue("numLesion", requestParams));
            if (consultaFederado.isEmpty()) {
                ConsultarFederadoDAO.consultaFederado(consultaFederado);
            }
            result.put("code", "997");
            result.put("message", "Es necesario introducir parametros en la consulta");
            log.error(" El resultado de la consulta es : " + result.toString());
        return result;
    }

Any thoughs on how i could improve this isEmpty() method?

The DTO is an object with 9 Strings.

EDIT: isEmpty function improved based on Stephen comment

EDIT2:

The final action:

public Map execute(Map params) throws Exception {
        log.error(" *********************** Consultar Federados Action INI ********************** ");
        ConsultarFederado consultaFederado = new ConsultarFederado();
        Map result = new HashMap<>();
        Map requestParams = (Map) params.get("request_params");
            log.info("setValue(nombre, requestParams)" + getValueValidated("nombre", requestParams));
            consultaFederado.setNombre(getValueValidated("nombre", requestParams));
            consultaFederado.setApellidos1(getValueValidated("apellidos1", requestParams));
            consultaFederado.setApellidos2(getValueValidated("apellidos2", requestParams));
            consultaFederado.setNIF(getValueValidated("nif", requestParams));
            consultaFederado.setFecha(getValueValidated("fecha", requestParams));
            consultaFederado.setPoliza(getValueValidated("poliza", requestParams));
            consultaFederado.setEstado(getValueValidated("estado", requestParams));
            consultaFederado.setNumeroLesion(getValueValidated("numLesion", requestParams));
             if (!consultaFederado.isEmpty()) {
                result = ConsultarFederadoDAO.consultaFederado(consultaFederado);
             } else {
                 XWMException err = new XWMException();
                 err.addError("996", "Es necesario introducir parametros en la consulta");
                 throw err;
             }
            log.error("**************** RESULT_SEND : " + result.toString());
            return result;
    }

private String getValueValidated(String value, Map requestParams) {
    return Objects.toString(requestParams.get(value), "");
    }

And the object isEmpty() method:

    public boolean isEmpty() {
     return StringUtils.isBlank(nombre) && StringUtils.isBlank(apellidos1) && StringUtils.isBlank(apellidos2) && StringUtils.isBlank(NIF) 
            && StringUtils.isBlank(fecha) && StringUtils.isBlank(poliza) && StringUtils.isBlank(estado) 
            && StringUtils.isBlank(numeroLesion);
}

public boolean isEmptyGeneral() {
    return StringUtils.isBlank(nombre) && StringUtils.isBlank(apellidos1) && StringUtils.isBlank(apellidos2) && StringUtils.isBlank(NIF) 
           && StringUtils.isBlank(fecha) && StringUtils.isBlank(numeroLesion);

}

public boolean isEmptyEstado() {
    return StringUtils.isBlank(nombre) && StringUtils.isBlank(apellidos1) && StringUtils.isBlank(apellidos2) && StringUtils.isBlank(NIF) 
           && StringUtils.isBlank(fecha) && StringUtils.isBlank(numeroLesion);
}

public boolean isEmptyNombre() {
    return StringUtils.isBlank(apellidos1) && StringUtils.isBlank(apellidos2) && StringUtils.isBlank(NIF) && StringUtils.isBlank(poliza) 
           && StringUtils.isBlank(fecha) && StringUtils.isBlank(numeroLesion);

}

Finally i created 4 "isEmpty" methods to check a few special combinations inside my code.

Here is my logic:

   public static Map consultaFederado(ConsultarFederado dto) throws XWMException, Exception    {
    if (StringUtils.isNotBlank(dto.getPoliza()) && dto.isEmptyGeneral()) {
         throwParamsError();
    } 
    if (StringUtils.isNotBlank(dto.getEstado()) && dto.isEmptyEstado()) {
       throwParamsError();
    } 
    if (StringUtils.isNotBlank(dto.getNombre()) && dto.isEmptyNombre()){ 
        throwParamsError();
    }
    
    Map result = buscarFederados(dto);
    return result;
}

Thank you eveyone for the feedback, without you this code would be quite worse

Grismak
  • 192
  • 16
  • https://stackoverflow.com/questions/56072593/easiest-way-to-check-if-property-is-null-or-empty-in-java Isn't this question similar? – SebastianJ Aug 24 '22 at 11:20
  • 1
    You shouldn't be testing strings using `==`. Not even against `""`. Write yourself a helper method like this `boolean isEmpty(String s) { return s != null && s.isEmpty(); }`. See [How do I compare strings in Java?](https://stackoverflow.com/questions/513832). – Stephen C Aug 24 '22 at 11:27
  • Will do Stephen, but i think that the base problem doesn´t change, in the end i am accesing all my parameters and checking if they are or not empty in quite a primitive way. – Grismak Aug 24 '22 at 11:30
  • @Grismak yes, but you do so in a way that is very likely to result in false negatives. – Stultuske Aug 24 '22 at 11:31
  • Maybe instead of checking the DTO, loop over the map entries and check if they are empty? You gain some dynamicism from Map so you might use it – Clashsoft Aug 24 '22 at 11:32
  • hm... that one is actually quite interesting @Clashsoft thanks, it seems that i could work with that, changing the focus seems like a good idea – Grismak Aug 24 '22 at 11:34
  • why would you give everything the value requestParams, btw ? – Stultuske Aug 24 '22 at 11:39
  • Is the architecture of my company, this is part of an API get call, and we work in actions where all params go inside the request_params. I cann´t modify that @Stultuske Currently i am looking into the solution proposed by clashsoft, it looks quite clean, for now – Grismak Aug 24 '22 at 11:41
  • 2
    The primitive way is the best way IMO. Certainly, the code is simple, readable and ... as efficient as it is likely to ever be. (There are other ways that may look elegant, but under the hood they are significantly less efficient.) Anyway ... if you want us to give you "better" solutions, you need to specify your criteria for "goodness". Otherwise this is just an opinion question. – Stephen C Aug 24 '22 at 11:44
  • I am quite in line with your thoughs Stephen, my major criteria is how efficient it is, and if there is a better solution. I think that as you say, there are roundabouts solutions that look cleaner but perform worse that this one. I just wanted to check if i am not missing something. – Grismak Aug 24 '22 at 11:53
  • Does `setValue("nombre", requestParams)` get (not set) the field `nombre` from requestParams? – k314159 Aug 24 '22 at 12:02
  • it works @k314159 it returns the name – Grismak Aug 24 '22 at 12:08
  • 1
    Then why is it called **set**Value instead of **get**Value? – k314159 Aug 24 '22 at 13:18
  • you are rigth @k314159 the naming is completely wrong, I will change it. Just to clarify this is what that function does: `private String setValue(String value, Map requestParams) { return requestParams.get(value) != null ? requestParams.get(value).toString() : ""; }` – Grismak Aug 25 '22 at 07:33
  • 1
    You can do it without calling `requestParams.get` twice: [`return Objects.toString(requestParams.get(value), "");`](https://docs.oracle.com/en/java/javase/18/docs/api/java.base/java/util/Objects.html#toString(java.lang.Object,java.lang.String)) – k314159 Aug 25 '22 at 10:01
  • Didn´t know that one @k314159 thanks, i will update the code when it´s finished with this changes. – Grismak Aug 25 '22 at 11:45

1 Answers1

0

If you want a nicer look one, how about a lombok @NonNull check, and adding string empty check for all field in a private constructor.

import lombok.Builder;
import lombok.Data;
import lombok.NonNull;
import org.apache.commons.lang3.StringUtils;


@Data
@Builder
public class TestDTO {
    @NonNull
    private String first;
    @NonNull
    private String second;
    @NonNull
    private String third;

    public static void main(String[] args) {
//        TestDTO dto = TestDTO.builder().first("fdasf").build(); //Exception in thread "main" java.lang.IllegalArgumentException: second can't be blank/empty/null
        TestDTO dto = TestDTO.builder().first("fdasf").second("fdasf").third("fdasfdsa").build();
        System.out.println("dto = " + dto);
    }

    private TestDTO(final String first, final String second, final String third) {
        if (StringUtils.isBlank(first)) {
            throw new IllegalArgumentException("First can't be blank/empty/null");
        }
        if (StringUtils.isBlank(second)) {
            throw new IllegalArgumentException("second can't be blank/empty/null");
        }
        if (StringUtils.isBlank(third)) {
            throw new IllegalArgumentException("third can't be blank/empty/null");
        }

        this.first = first;
        this.second = second;
        this.third = third;
    }
}


Hi computer
  • 946
  • 4
  • 8
  • 19
  • well, i can not use lombok in this project. Also, it´s not a problem if one or two parameters are blank the thing was to seach for a more efficient method that check the whole object. That being said, as Stephen clarified in the comments, i think that the solution i currently have is a good one. Thanks for you answer thought, is a clean one – Grismak Aug 25 '22 at 07:38
  • 1
    yeah, readability and simple is very important. just mention one thing, since it seems a lot of fields, you should consider builder pattern anyway. Thus you get flexible constructors and probably reduce the inconsistency in multi thread situation. https://stackoverflow.com/a/1953567/16702058 – Hi computer Aug 25 '22 at 09:53