I have the following Java class:

public class MyClass{

    private List<Settings> settings;

    public static class Settings {
        private String name;
        private List<Features> features;
    }

    public static class Features {
        private String name;
        private Boolean isActive;
    }
}

What I want to do is first check that settings is not null or empty. If not, then I want to find the Settings object that has the name "reliability", and then find its Features objects that have the names "logs" and "score" and get the isActive from these two objects.

This is what I've tried:

MyClass myClass = new MyClass ();
Boolean logs = false;
Boolean score = false;

if (myClass.getSettings != null) {
    for (Settings setting: myClass.getSettings) {
        if (setting.getName().equals("reliability")) {
            for (Features features : setting.getFeatures) {
                if (features.getName().equals("logs")) {
                    logs = features.getIsActive;
                } else if (features.getName().equals("score")) {
                    score = features.getIsActive;
                }
            }
        }
    }
}

How do I do this in a clean way? I can only do it with countless nested if and for loops, and it is not pretty.

CodePudding user response：

Here is the possible solution with Streams.

I assume that there would be no duplicated Features (i.e. having the same name) objects.

By the way, class names are usually singular nouns. Class Features is meant to represent a single object with a distinct name and a single property isActive. Therefore, the name Feature might` be more suitable.

The method below expects an argument of type MyClass, settings name and varargs of names of target features. The result it produces is Map with feature names as keys and corresponding isActive properties as values.

public static Map<String, Boolean> getFeaturesByName(MyClass myClass,
                                                     String settingName,
                                                     String... featureNames) {
    
    if (myClass.getSettings() == null) return Collections.emptyMap();
    
    Set<String> featureSet = Set.of(featureNames);
    
    return myClass.getSettings().stream()
        .filter(settings -> settings.getName().equals(settingName))
        .flatMap(settings -> settings.getFeatures().stream())
        .filter(features -> featureSet.contains(features.getName()))
        .collect(Collectors.toMap(
            MyClass.Features::getName,
            MyClass.Features::getActive
        ));
}

CodePudding user response：

Delegate the "drilling" to your classes:

add to MyClass

public boolean hasSettings() {
    return settings != null && !settings.isEmpty();
}

public Settings getSetting(String name) {
    return settings.stream()
            .filter(s -> s.hasName(name))
            .findFirst()
            .orElseThrow(() -> new IllegalStateException("No settings with name "   name));
}

add to Settings

public boolean hasName(String name) {
    return this.name.equals(name);
}

public Features getFeature(String name) {
    return features.stream()
            .filter(f -> f.hasName(name))
            .findFirst()
            .orElseThrow(() -> new IllegalStateException("No feature with name "   name));
}

add to Features

public boolean hasName(String name) {
    return this.name.equals(name);
}

Then you can do

if (myClass.hasSettings()) {
    Settings reliabilitySetting = myClass.getSetting("reliability");
    logs = reliabilitySetting.getFeature("logs").isActive();
    score = reliabilitySetting.getFeature("score").isActive();
}

NOTE: You can return Optionals if you don't want to throw exceptions.

CodePudding user response：

Well to check if the object is nullable or not you can use the optional class then regarding of find the setting object you have an option to use Java Stream Filter to filter the result.

CodePudding user response：

I think, a more fitting datastructure would be the Map. I presume every Features exists only once?

public class MyClass {

    private Map<String, Settings> settings;

    public static class Settings {
        private String name;
        private Map<String, Features> features;
    }

    public static class Features {
        private String name;
        private Boolean isActive;
    }

    public static void main(String[] args) {
        MyClass t = new MyClass();
        Boolean logs = null, score = null;
        Settings s = t.settings.get("reliability");
        if (s != null) {
            Features f;
            if ((f = s.features.get("logs")) != null) // Is "features" null-save?
                logs = f.isActive;
            else if ((f = s.features.get("score")) != null)
                score = f.isActive;
        }
        System.out.println("Logs: "   logs   ", Score: "   score);
    }
}

(Also it is very bad practice to use the nullable wrapperclass Boolean. You might wanna do something about that.)

If you have problems turning your existing lists into maps, use this method:

public static void main(String[] args) {
    Map<String, Settings> map = toMap(List.of(new Settings()), s -> s.name);
}

static <K, V> Map<K, V> toMap(Collection<V> c, Function<V, K> func) {
    return c.stream().collect(Collectors.toMap(func, v -> v));
}

CodePudding user response：

You can stream API to reduce the code

private static final String SETTINGS_NAME_FOR_CHECK = "reliability"; 
private static final List<String> FEATURES_NAME_FOR_CHECK = Arrays.asList("logs", "score"); 

public static void main(String[] args) {

    MyClass myClass = new MyClass();

    if(myClass.getSettings() != null) {

        Settings correctSettings = myClass.getSettings().stream()
                .filter(setting -> SETTINGS_NAME_FOR_CHECK.equals(setting.getName()))
                .findFirst().orElse(null);

        if(correctSettings.getFeatures() != null) {
            List<Features> features = correctSettings.getFeatures().stream()
                    .filter(feature -> (FEATURES_NAME_FOR_CHECK.contains(feature.getName()))).collect(Collectors.toList());

            System.out.println(features);
        }
    }
}

Once you get the features (which will be only having logs and shares), you can check for the boolean value.

CodePudding user response：

To make your code cleaner you can move logic into support methods. This also makes it more testable. In your code you are inspecting an instance of MyClass to determine if it certain features which are identified by name. You could write a method that does just that. Your original code could be re-written as:

MyClass myClass = ...
boolean hasLogs = hasSettingFeature(myClass, "reliability", "logs");
boolean hasScore = hasSettingFeature(myClass, "reliability", "score");

You can iterate through the given instance within the support model. You can do with this for-loops.

public boolean hasSettingFeature(MyClass myClass, String settingName, String featureName) {
  if (null == myClass || null == myClass.getSettings()) {
    return false;
  }
  for (Settings settings : myClass.getSettings()) {
    if (settingName.equals(settings.getName()) {
      for (Features features : settings.getFeatures()) {
        if (featureName.equals(features.getName()) {
          return true;
        }
      }
    }
  }
  return false;
}

You may also use the Stream API to filter to determine the state:

public boolean hasSettingFeature(MyClass myClass, String settingName, String featureName) {
  if (null == myClass || null == myClass.getSettings()) {
    return false;
  }
  return myClass.getSettings().stream()
      .filter(setting -> settingName.equals(setting.getName()))
      .flatMap(setting -> setting.getFeatures())
      .filter(features -> featureName.equals(features.getName()))
      .findAny()
      .isPresent();
}