1
\$\begingroup\$

I have a list of Students and I want group this list by the property NAME, to sort each group ASC by the property GRADE, to remove the first entry from each Group and after thatto return the proccesed list.

Here is what I've done.

package com.example.demo;

import java.security.KeyStore.Entry;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

public class Test {
    public static void main(String[] args) {

        Student studentA2 = new Student("A", 1);
        Student studentA1 = new Student("A", 2);
        Student studentA3 = new Student("A", 3);
        Student studentA4 = new Student("A", 5);
        Student studentA5 = new Student("A", 9);
        Student studentB4 = new Student("B", 0);
        Student studentB1 = new Student("B", 10);
        Student studentB2 = new Student("B", 4);
        Student studentB3 = new Student("B", 2);

        List<Student> allStudents = new LinkedList<>();

        allStudents.addAll(Arrays.asList(studentA1, studentA2, studentA3, studentB1, studentB2, studentB3, studentA4,
                studentA5, studentB4));

        List<Student> procesedList = doGroupValuesAndRemoveTheSmallestGrade(allStudents);

        // The entry: Student("A", 1) and new Student("B", 0) are removed;
        procesedList.forEach(x -> System.out.println(x.name + " " + x.grade));
    }

    private static List<Student> doGroupValuesAndRemoveTheSmallestGrade(List<Student> allStudents) {
        Collections.sort(allStudents, Comparator.comparing(Student::getName).thenComparing(Student::getGrade));

        Map<String, LinkedList<Student>> groupedStudents = new HashMap<>();

        for (Student student : allStudents) {

            boolean isNewGroup = groupedStudents.get(student.getName()) == null;

            if (isNewGroup) {
                groupedStudents.put(student.getName(), new LinkedList<>());
            }

            groupedStudents.get(student.getName()).add(student);
        }

        groupedStudents.forEach((k, v) -> v.removeFirst());

        List<Student> procesedList = new ArrayList<>();

        groupedStudents.forEach((k, childList) -> childList.forEach(student -> procesedList.add(student)));
        return procesedList;
    }

}

class Student {
    public String name;
    public int grade;

    public Student(String name, int grade) {
        super();
        this.name = name;
        this.grade = grade;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public int getGrade() {
        return grade;
    }

    public void setGrade(int grade) {
        this.grade = grade;
    }

}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

No point in having named individual variables for your Student instances - add them to the list inline. Also, don't construct the list directly; just accept the result of asList.

Make Student a record.

Student is not a great name for what actually appears to be a StudentGrade.

doGroupValuesAndRemoveTheSmallestGrade is a painful name. You're better off abbreviating this to something like groupAndTrim, and adding meaningful javadocs for the details.

doGroupValuesAndRemoveTheSmallestGrade can be re-expressed in streams, and would then return a stream instead of a list.

You should replace println with printf. Better yet, do it in memory by formatting strings and then sending them to a single print.

main should go away and be replaced with an actual unit test.

Suggested

There are probably better places to put groupAndTrim, but those depend on the broader context of "what you're actually doing" which you have not shown. In the absence of a fully-defined data processing pipeline I have shown this as a static on StudentGrade itself.

Demonstrating some of the above,

package com.stackexchange.studentgroup;

import java.util.Collection;
import java.util.Comparator;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public record StudentGrade(
    String name,
    int grade
) {
    
    @Override
    public String toString()
    {
        return String.format("%s %d", name, grade);
    }

    /**
     * Sort by student name and then by grade. Drop each student's lowest grade.
     */
    public static Stream<StudentGrade> groupAndTrim(Collection<StudentGrade> grades) {
        return grades.stream()
            .collect(Collectors.groupingBy(
                StudentGrade::name
            ))
            .values().stream()
            .flatMap(
                students ->
                    students.stream()
                        .sorted(
                            Comparator.comparing(StudentGrade::grade)
                        )
                        .skip(1)
            );
    }
}
package com.stackexchange.studentgroup;

import java.util.Arrays;
import java.util.List;

import static org.junit.jupiter.api.Assertions.*;
import org.junit.jupiter.api.Test;


public class GroupTest {
    @Test
    public void testGroupTrim() {

        List<StudentGrade> grades = Arrays.asList(
            new StudentGrade("A", 2),
            new StudentGrade("A", 1),
            new StudentGrade("A", 3),
            new StudentGrade("B", 10),
            new StudentGrade("B", 4),
            new StudentGrade("B", 2),
            new StudentGrade("A", 5),
            new StudentGrade("A", 9),
            new StudentGrade("B", 0)
        );

        List<StudentGrade> processed = StudentGrade.groupAndTrim(grades).toList();

        // The entry: Student("A", 1) and new Student("B", 0) are removed

        assertEquals(7, processed.size());

        for (StudentGrade grade: grades) {
            boolean copied = processed.contains(grade);

            int shouldDrop = switch(grade.name()) {
                case "A" -> 1;
                case "B" -> 0;
                default -> throw new UnsupportedOperationException();
            };

            assertTrue((grade.grade() == shouldDrop) ^ copied);
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.