Nsryjdtyk

I have written the following code for one of my requirements. Kindly help me if I can write this logic in a better.

public class PersonService {

    public PersonCategorized getCategorizedPerson(List<Person> allPersonFromDb) {

        List<Person> adultList = allPersonFromDb.stream().filter(PersonUtil::isAdult).collect(Collectors.toList());

        List<Person> underAgedList = allPersonFromDb.stream().filter(PersonUtil::isUnderAged).collect(Collectors.toList());

        List<Person> seniorCitizenList = allPersonFromDb.stream().filter(PersonUtil::isSeniorCitizen).collect(Collectors.toList());



        return new PersonCategorized(underAgedList, adultList, seniorCitizenList);

    }

}



public class PersonCategorized {

    private List<Person> underAgedPersonList;

    private List<Person> adultList;

    private List<Person> seniorCitizenList;



    public PersonCategorized(List<Person> underAgedPersonList, List<Person> adultList, List<Person> seniorCitizenList) {

        this.underAgedPersonList = underAgedPersonList;

        this.adultList = adultList;

        this.seniorCitizenList = seniorCitizenList;

    }



    public List<Person> getAdultList() {

        return adultList;

    }



    public List<Person> getSeniorCitizenList() {

        return seniorCitizenList;

    }



    public List<Person> getUnderAgedPersonList() {

        return underAgedPersonList;

    }

}



public class Person {

    private int age;

    private String name;



    public Person(String name, int age) {

        this.name = name;

        this.age = age;

    }



    public int getAge() {

        return age;

    }



    public String getName() {

        return name;

    }

}



public class PersonUtil {

    public static boolean isAdult(Person person) {

        return person.getAge() >= 18 && person.getAge() < 60;

    }



    public static boolean isUnderAged(Person person) {

        return person.getAge() >= 0 && person.getAge() < 18;

    }



    public static boolean isSeniorCitizen(Person person) {

        return person.getAge() >= 0 && person.getAge() < 18;

    }

}

One observation I had after posting here is I am iterating over the list 3 times to collect 3 different lists.

No much room for improvement here. The code is clear, readable, maintainable, short and nice. You could get rid of the three iterations by using a loop and filling three lists all in one iteration, but you'd sacrifice maintainability for a few microseconds saved. Don't do this unless you are in a real critical spot. For real world applications, keep it as it is.

One thing however is the PersonUtil. From an OO perspective, the Person should probably know whether it is underaged or senior or whatever and contain that business logic itself, so that you don't have to resort to a util-class. However, if the Person is an entity, eventually generated, you basically have no other choice.

Always remember to use the access modifiers judiciously. From the question as the package hierarchy is unknown, I assumed that the all the classes are in the same package.

public class PersonUtil {

  public static boolean isAdult(Person person) {

    return person.getAge() >= 18 && person.getAge() < 60;

  }



  public static boolean isUnderAged(Person person) {

    return person.getAge() >= 0 && person.getAge() < 18;

  }



  public static boolean isSeniorCitizen(Person person) {

    return person.getAge() >= 0 && person.getAge() < 18;

  }

}



public class Person {

  private String name;

  private int age;



  Person(String name, int age) {

    this.name = name;

    this.age = age;

  }



  public int getAge() {

    return age;

  }



  public String getName() {

    return name;

  }

}



public class PersonCategorized {

  private List<Person> underAgedPersonList;

  private List<Person> adultList;

  private List<Person> seniorCitizenList;



  PersonCategorized(List<Person> underAgedPersonList, List<Person> adultList, List<Person> seniorCitizenList) {

    this.underAgedPersonList = underAgedPersonList;

    this.adultList = adultList;

    this.seniorCitizenList = seniorCitizenList;

  }



  public List<Person> getAdultList() {

    return adultList;

  }



  public List<Person> getSeniorCitizenList() {

    return seniorCitizenList;

  }



  public List<Person> getUnderAgedPersonList() {

    return underAgedPersonList;

  }

}







 public class PersonService {



      public PersonCategorized getCategorizedPerson(List<Person> allPersonFromDb) {

        try (Stream<Person> personStream = allPersonFromDb.stream()) {

          return new PersonCategorized(

                     collectFilteredList(personStream.filter(PersonUtil::isUnderAged)),

                     collectFilteredList(personStream.filter(PersonUtil::isAdult)),

                     collectFilteredList(personStream.filter(PersonUtil::isSeniorCitizen)));

        }

      }



      private List<Person> collectFilteredList(Stream<Person> filteredStream) {

        assert filteredStream == null;

        return filteredStream.collect(Collectors.toList());

      }



    }

Naming for me seems to be not so okay check for singular and plural wherever required.

If conditions are exclusive you could use enum:

class PersonService {



    public Map<PersonCategory, List<Person>> getCategorizedPerson(List<Person> allPersonFromDb) {

        return allPersonFromDb.stream().collect(Collectors.groupingBy(PersonUtil::getCategory));

    }

}



enum PersonCategory {

    UNDERAGE,

    SENIOR,

    DEFAULT

}



class Person {



    private int age;

    private String name;



    public Person(String name, int age) {

        this.name = name;

        this.age = age;

    }



    public int getAge() {

        return age;

    }



    public String getName() {

        return name;

    }

}



class PersonUtil {



    public static PersonCategory getCategory(Person person) {

        if (isUnderAged(person)) {

            return PersonCategory.UNDERAGE;

        }

        if (isSeniorCitizen(person)) {

            return PersonCategory.SENIOR;

        }

        return PersonCategory.DEFAULT;

    }



    public static boolean isUnderAged(Person person) {

        return person.getAge() >= 0 && person.getAge() < 18;

    }



    public static boolean isSeniorCitizen(Person person) {

        return person.getAge() >= 0 && person.getAge() < 18;

    }

}