Load and analyze a list of people from a file

I recently completed a programming task in Java for a job that was javascript heavy but the hiring company wanted some Java knowledge. I've been using Java since the turn of the year. Other than learning on the job I've read Clean Code and thought I could have a go at this task. Clearly I failed because the recruitment agent is avoiding me. I thought my design was ok but I'm going wrong somewhere/everywhere so want to learn from this.

The task was:


INSTRUCTIONS


Please carefully read these instructions before attempting to write
your solution.


This is an exercise to write some java code to read in a text file and
answer three simple questions. The questions are at the bottom of this
file. Once you have finished the exercise, please zip this directory
and email it back to us. Please note that we:



-
are looking for code that is written in a modular way that demonstrates thought about reusability, maintainability and your
understanding of object oriented programming.

-
are looking for you to showcase your code design skills, so feel free to create the necessary classes, avoid long procedural classes,
overuse of static members, messy code, etc.

-
have provided IntelliJ IDEA & Eclipse project files for your convenience, but you can ignore, delete or replace them.



QUESTIONS


Please read in the data contained within the file manipulate-data.txt


Manipulate the data to programmatically answer the following
questions:

• How many people in the list are male?

• In years what is the average age of the people in the list?

• How many days older is Jeff Briton than Tom Soyer?

Please print your answers out to the screen by using the
'System.out.print' function.

My code:

This class was provided with some code in and a private method. I wanted to keep this really clean (which might have been where I went wrong - see my own critique of my code below).

```
public class ManipulateDataApplica

Design Princples and Patterns

Other than learning on the job I've read Clean Code

Uncle Bob would wag his finger at you, then open up that book to SRP. :D

I created a class for the list of Person objects but looking back I didn't separate the data from the object and instead thought that this class will include everything for working with the list so probably breaking the single responsibility principle.

Yes. You are correct. making just the list class which does only list things would have been the correct approach, anything else breaks SRP.

so a bit more of an OO approach would have been to have a factory that takes in a path (be it in string form or other) and returns your person list. Its only responsibility would be reading the file end to end and creating a list with all the people in it.

And finally the Person object (I'm not really confident about the way I've used multiple constructors here):

and that is a good concern to have. Here there are a few approaches, you could again go with the factory pattern that takes in a line from the file from your other factory which is reading the file. You could make a converter/adapter (same concept just coded slightly differently) The factory pattern again seems to fit the bill the best.

those 4 classes would have gotten you to the point of having all the objects you need. I would not have gone past that. This makes them modular and easy to use else where because they behave much like any other class in the java world. So when it comes down to answering the 3 questions again you would want to create another class that does those things. I think the Builder design pattern would be a good choice here. (sorry for the C# version, but the same principle applies) GOF builder design pattern

I wont implement the code, but show you what structure I would follow. First I would make the builder interface

public interface FindAnswerToQuestion {
    void findAnswer(PeopleList peopleList);
    String getAnswer();
}

then I'd make a class that implements it for one of the questions

public class FindCountOfMalesAnswer implements FindAnswerToQuestion {
    @Override
    public void findAnswer(PeopleList peopleList) {

    }

    @Override
    public String getAnswer() {
        return "";
    }
}

From that point using it is very simple

FindAnswerToQuestion[] finders = {
            new FindCountOfMalesAnswer(),
    };
    for (FindAnswerToQuestion finder : finders){
        finder.findAnswer(peopleList);
        System.out.println(finder.getAnswer());
    }

it would be debatable in my opinion to make the concrete builders return the answer in English or to just return the numbers and let others format it. The way I see it it would be trivial to switch back and forth because on findAnswer() I would think you'd save the number internally on that class then in the getAnswer() you would either return the number or the formatted string.
Other concerns

public List buildPeopleListFromFile(String fileName) throws FailedToBuildPeopleListException {
    File file = new File(fileName);
    System.out.println(file.getName() + " file exists = " + file.exists());
    try (BufferedReader bufferedReader = Files.newBufferedReader(Paths.get(fileName))) {
        int lineNumber = 0;
        String line;
        while ((line = bufferedReader.readLine()) != null) {
            if (lineNumber > 0) {
                Person person = new Person(line);
                peopleList.add(person);
            }
            lineNumber++;
        }
    } catch (IOException e) {
        throw new FailedToBuildPeopleListException(e);
    }

    return peopleList;
}

in this you count the line numbers. It isn't part of the requirements to know this, and you don't use it anywhere...so why? You also used Files to get a BufferedReader but you are really only concerned with each line in the file. so using the stream feature of Java 8 you could simplify reading the file and adding to your list like so

Files.lines(Paths.get(fileName)).forEach(line-> peopleList.add(new Person(line)));

public Map countBySex() throws InvalidPeopleListException {
    if (!peopleList.isEmpty()) {
        Map counted = peopleList.stream()
                .collect(Collectors.groupingBy(Person::getSex, Collectors.counting()));

        return counted;
    } else {
        throw new InvalidPeopleListException("PeopleList is empty");
    }
}

I like this concept that you make it easy to get a count of either gender... but a string??? what would happen if you asked for "Male", "MALE", "M" or any other combination? What if the file itself is not standardized. This would have been much better as an Enum. then you can parse the file and know that you have a type safe version of it to group by.

On all of your methods you test for a condition (!peopleList.isEmpty()) do your business then get out. The problem with this is that I have read all the logic to find out that if

StackExchange Code Review Q#134793, answer score: 5