Famous M. Ighodaro

Campbell Ritchie wrote:Why have you made the Enrollment class a singleton? I don't think that is the correct way to write a singleton anyway.

The reason of the Singleton is that the enrollment class is holding the list of enrolled students and lessons. To print out the list of enrolled students and lessons later from another class will possible by calling just one instance of the enrollment class.
I choose this way of writting Singleton because the assignment is not a muiltithreaded application.

Junilu Lacar wrote:And here's my Student class implementation based on the tests I have written so far:
import java.util.HashSet; import java.util.Set; public class Student {    private Set<Course> coarseLoad = new HashSet<>();    public boolean enrollIn(Course newCoarse) {        if (coarseLoad.contains(newCoarse)) {            return false;        }        return newCoarse.addToRoster(this);    }    public boolean isTaking(Course course) {        return coarseLoad.contains(course);    }    void synchronizeEnrollment(Course course) {        if (course.isOnRoster(this)) {            coarseLoad.add(course);        } else {            coarseLoad.remove(course);        }    } }
synchronizeEnrollment(Course) is the callback method I alluded to earlier in this thread. It has package-private (default) scope by design.

I like how you simplified your code. My concern is that, the student class is maintaining a list of enrolled lessons and also updating the lesson object by adding a student to the new course. Won't this be breaking single responsibility principle or I am getting the idea of single responsibility principle all wrong? I am still learning, so explanation if what I am getting wrong will be appreciated.

Junilu Lacar wrote:Previously,

I wrote:there is at least one logic bug

The design has a number of problems related to breaking the encapsulation of Student and Lesson and misplacing the responsibility of enforcing the following rules:

1. A student can only enroll in a limited number of courses. (Apparently, no more than 2)

2. A course/lesson can only accept a limited number of students as determined by Lesson.getCapacity()

Consider what happens when a student tries to enroll in a course that is already at capacity. I haven't tested it but from what I can tell by just looking at the code, even though a student doesn't get added to a lesson because it is already at full capacity, the lesson will still get added to the student's list of lessons taken as long as they haven't yet enrolled in the maximum number of lessons allowed per student.

Thanks for the feedback and thorough analysis of my code above line by line. I found out this bug a few days back and this is how I handled it. Please I any critique is always welcome. I added a removeStudent method to handle it.



import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; /** * * @author Famous Ighodaro Monday */ public class Enrollment {            private  Map<Student, Set<Lesson>> enrolledStudents;    private  Map<Lesson, Set<Student>> enrolledlessons;    private static Enrollment instance;    private Error error;        private  Enrollment(){        enrolledStudents = new HashMap<>();        enrolledlessons = new HashMap<>();    }      public static Enrollment getInstance(){      if (instance == null) {          instance = new Enrollment();      }      return instance;  }        public Map<Student, Set<Lesson>> getEnrolledStudents(){                return enrolledStudents;    }        /**     * @return boolean if the lesson was added to to student to enrolled course successfully.     * @param Lesson type     * @param Student type     * The add lesson method, add a lesson to student enrolled list.     *     */    public boolean addLesson(Lesson lesson, Student student){        boolean isLessonAdded=false ;          if (enrolledlessons.containsKey(lesson)){            Set<Student> students = enrolledlessons.get(lesson);            if (!students.contains(student) && students.size()<lesson.getCapacity()) { >                students.add(student);                isLessonAdded=true;            }else if(students.contains(student)){                error = Error.DUPLICATE;                isLessonAdded =  false;                            }else{                error = Error.LESSON_FULL;                isLessonAdded = false;            }        }else{            Set<Student> students = new HashSet<>();            students.add(student);           enrolledlessons.put(lesson, students);           isLessonAdded=true;        }       return isLessonAdded;    }            public Map<Lesson, Set><Student>> getEnrolledLessons(){      return enrolledlessons;    }         /**     * @return boolean if a lesson was added  to student to enrolled course successfully.     * @param Lesson type     * @param Student type     * The addStudent method, add a student to the map if it does not exit on the map yet,     * else it only add the lesson to the student list if he or she has not enrolled in more than two lesson     * and also they haven't enrolled in the course before.     *     */     public boolean addStudent(Student student, Lesson lesson){         int enrolledLessonCount = student.getNumberOfLessonsEnrolled();        boolean isStudentAdded=false ;          if (enrolledStudents.containsKey(student)){            Set<Lesson> lessons = enrolledStudents.get(student);                      if (!lessons.contains(lesson) && student.getNumberOfLessonsEnrolled()<2) {                lessons.add(lesson);                              System.err.println("number of lesson enrolled before:- "+ enrolledLessonCount);                enrolledLessonCount++;                student.setNumberOfLessonsEnrolled(enrolledLessonCount);                 System.err.println("number of lesson enrolled after:- "+ student.getNumberOfLessonsEnrolled());                System.err.println("sucess");                 isStudentAdded=true;            }else if(lessons.contains(lesson)){                              isStudentAdded =  false;                System.err.println("It already contains this lessons");                            }else{                System.err.println("You can't enroll more than two course a week");                error = Error.LESSON_FULL;                isStudentAdded = false;            }        }else{            System.err.println("first enrollment");            Set<Lesson> lessons = new HashSet<>();            lessons.add(lesson);           enrolledStudents.put(student, lessons);           isStudentAdded=true;           enrolledLessonCount++;           student.setNumberOfLessonsEnrolled(enrolledLessonCount);        }       return isStudentAdded;    }         // I added this new method to remove a student from enrolledLessons list so that if the addStudent was not successful     public void removeStudent(Lesson lesson, Student student){         if(enrolledlessons.containsKey(lesson)){             Set<Student> students = enrolledlessons.get(lesson);             if(students.contains(student)){                 students.remove(student);             }else{                 System.err.println("This student does not exit");             }         }else{             System.err.println("This Lesson does not exist");         }     }         public void enroll(Lesson lesson, Student student){         boolean isStudentAdd;         if(this.addLesson(lesson, student)){             isStudentAdd=this.addStudent(student, lesson);             if(isStudentAdd){                 System.out.println("Enrollment successfull");             }else{                 this.removeStudent(lesson, student);                 System.err.println("You can't enrolled in more than 2 lesson");             }         }else{             System.err.println("Lesson capacity is already full and you are already enrolled in this lesson");         }             }     }

Junilu Lacar wrote:

Famous M. Ighodaro wrote:I want to be able to print each course later and the number of students enrolled on it. And also print all the student and the number of course each student enrolled in.

This is the part that most likely will lead to breaking the SRP. Printing a class roster and printing a coarse load are different responsibilities from maintaining enrollment information. If you have methods in a Course class that maintain the roster of students and print the roster of students taking the course then that is a violation of the SRP. Likewise in a Student class if you have methods that maintain information about course load and printing the coarse load. The responsibilities of maintaining the list and printing the list should be separated. Here's why: If you need to change the way the printed list is formatted, ordered, or filtered, it has nothing to do with how the list is maintained. Maintaining the list is therefore a different responsibility from printing the list.

Hello, that was the initial idea, I mentioned it on the comment above because I want it to be taking into consideration when any one is answering my question. The SRP I was concerned about is the student and the lesson class maintaining the enrolled lessons and enrolled students respectively.

Carey Brown wrote:I believe that would break the principle. You'd end up with Course having a List<Student> and Student having a List<Course>. Then the problem becomes going to two places to add a Student to a Course.
You could have an Enrollment class, for example, with Map<Student,List<Course>> and also Map<Course,List<Student>>. Then you only have to go to the Enrollment object to add a Student to a Course and both maps would be maintained in sync.

Edit: Probably "Set" would be better than List.

Thanks for your answer, I took your suggestion and this is how the Enrollment class look like now. I am not sure yet of how the enroll method should look like, but this is what I can put into work now.
package hartfield.leisure.center; import bookingApp.users.Student; import bookingApp.error.Error; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; public class Enrollment {            private  Map<Student, Set<Lesson>> enrolledStudents;    private  Map<Lesson, Set<Student>> enrolledlessons;    private static Enrollment instance;    private Error error;        private  Enrollment(){        enrolledStudents = new HashMap<>();        enrolledlessons = new HashMap<>();    }      public static Enrollment getInstance(){      if (instance == null) {          instance = new Enrollment();      }      return instance;  }        public Map<Student, Set<Lesson>> getEnrolledStudents(){                return enrolledStudents;    }        /**     * @return boolean if the lesson was added to to student to enrolled course successfully.     * @param Lesson type     * @param Student type     * The add lesson method, add a lesson to student enrolled list.     *     */    public boolean addLesson(Lesson lesson, Student student){        boolean isLessonAdded=false ;          if (enrolledlessons.containsKey(lesson)){            Set<Student> students = enrolledlessons.get(lesson);            if (!students.contains(student) && students.size()<lesson.getCapacity()) {                students.add(student);                isLessonAdded=true;            }else if(students.contains(student)){                error = Error.DUPLICATE;                isLessonAdded =  false;                            }else{                error = Error.LESSON_FULL;                isLessonAdded = false;            }        }else{            Set<Student> students = new HashSet<>();            students.add(student);           enrolledlessons.put(lesson, students);           isLessonAdded=true;        }       return isLessonAdded;    }            public Map<Lesson, Set<Student>> getEnrolledLessons(){      return enrolledlessons;    }         /**     * @return boolean if a lesson was added  to student to enrolled course successfully.     * @param Lesson type     * @param Student type     * The addStudent method, add a student to the map if it does not exit on the map yet,     * else it only add the lesson to the student list if he or she has not enrolled in more than two lesson     * and also they haven't enrolled in the course before.     *     */     public boolean addStudent(Student student, Lesson lesson){         int enrolledLessonCount = student.getNumberOfLessonsEnrolled();        boolean isStudentAdded=false ;          if (enrolledStudents.containsKey(student)){            Set<Lesson> lessons = enrolledStudents.get(student);                      if (!lessons.contains(lesson) && student.getNumberOfLessonsEnrolled()<2) {                lessons.add(lesson);                    enrolledLessonCount++;                student.setNumberOfLessonsEnrolled(enrolledLessonCount);                                System.err.println("sucess");                 isStudentAdded=true;            }else if(lessons.contains(lesson)){              error.Error.DUPLICATE;                isStudentAdded =  false;                                            }else{                            error = Error.LESSON_FULL;                isStudentAdded = false;            }        }else{            Set<Lesson> lessons = new HashSet<>();            lessons.add(lesson);           enrolledStudents.put(student, lessons);           isStudentAdded=true;           enrolledLessonCount++;           student.setNumberOfLessonsEnrolled(enrolledLessonCount);        }       return isStudentAdded;    }             public void enroll(Lesson lesson, Student student){         this.addLesson(lesson, student);         this.addStudent(student, lesson);     }     }

I am working on an assignment project, I have a student class which has a variable such as name and address. Also there is a Course class which has variable such as couseName, teacher, date, startTime and endTime. Will I be breaking the single responsibility if I add an ArraylList instance variable to the Course class to maintain the list of student that enrolled on the course? This question  arises after reading the single responsibility principle. I want to be able to print each course later and the number of students enrolled on it. And also print all the student and the number of course each student enrolled in.