eclipse / microprofile-health

microprofile-health
Apache License 2.0
107 stars 61 forks source link

data field of type Optional<Map<String, Object>> #323

Closed gquintana closed 2 months ago

gquintana commented 1 year ago

Having a field of type Optional<Map<String, Object>> does not make sense to me, When no data has been set, returning a empty Map instead of an empty Optional is far simpler and clearer. Wrapping a Collection in an Optional is not considered as a good practice:

At least could you add helper methods like

    public Map<String, Object> getDataMap() {
        return getData().orElse(Collections.emptyMap());
    }
    public Optional<Object> getData(String key) {
       return getData().map(data -> data.get(key));
    }
HerrDerb commented 2 months ago

yes, we just ran into an unexpected NPE because of this.

  1. Optional as a field is a very bad smell.
  2. It is even worse if the field additionally get initialized with null.

This code should be refactored