1

I'm developing a hobby project to properly understand encapsulation, what classes can be responsible for, and rules. I asked for a code review and assistance in another forum, but I don't agree with the approach given.

I have the following requirements:

  • An international student requires documents to complete the registration process, but domestic students don't.

StudentStatus Interface:

public interface StudentStatus {
    Collection<String> retrieveDocuments();
    StudentType retrieveStatus();
}

 public final class Domestic implements StudentStatus {

       private final StudentType type;
       private final Collection<String> documents;

       public Domestic() {
           this.type = StudentType.Domestic;
           this.documents = Collections.emptyList();
       }

       @Override
       public Collection<String> retrieveDocuments() {
           return this.documents;
       }

       @Override
       public StudentType retrieveStatus() {
           return type;
       }
   }

public final class International implements StudentStatus {

   private final StudentType type;
   private Collection<String> documents;


   public International(Collection<String> documents) {

       this.type = StudentType.International;
       this.documents = Collections.unmodifiableCollection(documents);
   }

   @Override
   public Collection<String> retrieveDocuments() {
       return Collections.unmodifiableCollection(documents);
   }

   @Override
   public StudentType retrieveStatus() {
      return type;
   }
}

Student class:

public final class Student {

     //left out constructor and getters for other attributes. 

     public Collection<String> retrieveDocuments() {
           return status.retrieveDocuments();
     }

     public StudentType retrieveStatus() {
          return status.retrieveStatus();
     }

     public boolean isVerified(StudentType type) {
         return this.retrieveStatus() == type;
     }
}

University class:

public class University {

    private final Map<Student,Collection<String>> registeredStudents;
    private final StudentType type;

    public University()
    {
        registeredStudents = new HashMap<Student,Collection<String>>();
        type = StudentType.International;
    }

    public void add(Student student){
        if (student.isVerified(type)){
            registeredStudents.put(student, student.retrieveDocuments());
        }else {
            //throw an exception or handle error accordingly 
        }
    }
}

Before I continue, I understand that this is a really over simplified application process. In the real world, a lot more has to happen before a Student can register. The student may have to go through entrance exams, and payment before registration begins. Also, in a realistic environment, this information would probably be stored in a database that the campus employees can access.

In the other forum, the conversation went into what information is being given out, and approaches were given.

  • Have a rule class, that takes the Student object and verifies that it is in fact international and has documents.

The problem I have with this, is you're still going to have to ask the Student his/her status either with the retriveStatus() or isVerified(), I don't really see how to do it any other way.

  • Pass the Student and collection of documents separately to be added to the Map.

In the real world, the University set the rule as stated above and it's responsibility is to check if International students have documentation.

When I suggested the approach above with the add(Student student) they stated it wasn't a good idea because the rules can change, and you'll have to change the Student class as well as the University class.

However, in the real world, a student is well aware of his/her status and if he/she is domestic/international and in possession of documents that can be given to the school.

Given the above approach, is writing the add method this way a good idea? Is there a better approach than the add method?

tl;dr - If a Student has to follow the rules set by the University, how then would the Student object communicate with the University to get the data so that the University can ensure the student object is complying with the rules without breaking encapsulation?

  • 1
    I think you need some changes in the Model. Consider what dependencies should be present in each Class. For example should Student object be aware of status? Here is my suggestion: Create abstractions for Student, Status, Document ,University, and RegistrationStrategy. You can either use Abstract Factory for various registration processes or use Strategy. – Amit Mahajan Oct 30 '17 at 21:52
  • Does a university enrol students of one type? – Andrew Tobilko Oct 30 '17 at 22:07
  • @AndrewTobilko - No, but I wanted to focus on international because that's the one with the set of rules I'm having trouble understanding and implementing. –  Oct 30 '17 at 22:36
  • @amitmah - From a real world perspective, a Student is aware of status, you are aware if you are domestic or international and have documents to prove your identity. I don't see why a Student would not be aware of that. –  Oct 30 '17 at 22:37
  • the `isVerified` shouldn't be in the `Student`. A student can't confirm his verification. There can be some services involved in this validation. Will you bind the entity to these services? But a student can give documents to a service which is in charge of deciding whether these documents meet the requirements. A student knows nothing about the requirements as well as the structure of documents he owns. – Andrew Tobilko Oct 31 '17 at 11:49
  • @AndrewTobilko - Completely agree here. Now bare with me. This means, that `Student` has data that the school wants(first/last name, residence, and of course, documents). In the real world the school asks for these and a student produces them. In code, this means the university class calls the `getters` that `Student` provides to grab the data correct? I'm not breaking encapsulation, right? When you say structure of documents, this means don't provide List documents, rather Collection documents, right? –  Oct 31 '17 at 12:48
  • @S.R. Right. Getters here support encapsulation revealing the data a student wants to show. It should not necessarily be returning a private field. Documents could pass through filters (some kind of projection)[depending on a Student implementation] before they get published. – Andrew Tobilko Oct 31 '17 at 13:07
  • @AndrewTobilko - When you say " It should not necessarily be returning a private field" what do you mean? because first/last name, and documents are all declared private and `getters` are provided to access them. Could you provide an example of how you would do it? –  Oct 31 '17 at 13:10
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/157915/discussion-between-andrew-tobilko-and-s-r). – Andrew Tobilko Oct 31 '17 at 14:03

2 Answers2

0

The conversation in previous post was probably leading you in a generally good direction. The principle that applies most is the Open / Closed principle. https://en.wikipedia.org/wiki/Open/closed_principle.

Don't set yourself up to have to constantly modify a particular class or set of classes (in OO world at least) in an area you know is going to be a frequent vector of change. The principle applies equally in the functional world, but your example is using an OOPL.

Little hand-built rules engine is a pretty good solution for your stated problem. Particularly if you know the rule flows on pretty fixed inputs - like the University and the Student. DocumentsRequiredForInternationalStudents is a rule class in that architecture - only needs to change if something about that rule itself changes. New rule, which is going to happen a lot = add new class, not modify existing one.

Sometimes you don't know vector of change, harder to make decisions, but if it's obvious, don't architect a system where you'll have to violate open/closed constantly due to an known change vector.

There are different ways to implement little rules engines. One option (this is crappy pseudo-code so it takes less space)

interface RegistrationRule
 boolean isRegistrationValid(Student student)  //might need university too for some rules.

class DocumentsNeededForInternationalStudents implements RegistrationRule
 boolean isRegistrationValid(Student student) 
 // return student.status is international and student has documents, or student status is domestic 
// (this rule passes through as valid any domestic students).

class RegistrationRules 
// (holds all the rules you will use - kind of a factory)
constructor -> add to static list of rules an instance of all your rules
boolean runRulesForStudent(Student)
  //iterate through all rules, call isRegistrationValid, short circuit and return false if one of them false


class University
addStudent(Student student)
  if (RegistrationRules.runRules(student).... else

That's just one way to throw it together, but you can see it's not really a lot of code. You have an interface, an implementation class for each rule, and a little rules engine class that applies each rule for you. New rule = new class, modify your constructor in the rules engine to add an instance of that class, done.

This pattern begins to struggle a bit when the properties and behavior that are needed in the rules are very diverse and not concentrated in the same small set of classes.

Jim Weaver
  • 983
  • 5
  • 15
  • If I follow your approach, and maybe it will help if I see an example, but if a new rule means a new class, this means, I'm asking for information from the Student to verify if he/she is following the rules. So if I had a rule call `DocumentsRequiredForInternationalStudents`, how might that method look like? do I call the `getters` from Student or provide `methods` within student to do the work? My problem is how does Student communicate with the class that is checking if it's following the rules of the University without breaking encapsulation? That's what I really want to know. –  Oct 30 '17 at 22:41
  • Edited above. You can call getters. Encapsulation is a good thing, but sometimes you do need to access properties on a class because doing something really should not be that classes responsibility. Registration is a great example. Should student be aware of an individual university's registration rules? Probably not. So that means some other unit of code needs information about the student to determine registration validity. Encapsulation and the single responsibility principle are often at odds with one another - you have to decide based on what fits your problem best. – Jim Weaver Oct 31 '17 at 15:38
  • It's almost how I abstract away `StudentStatus`(see updated code above). Given that the status has it's own set of rules to enforce, it makes sense to create a class an enforce those rules, rather than let Student beware of all the `StudentTypes` –  Oct 31 '17 at 15:49
  • Yep StudentStatus alone in your model might be all you would need to pass to a rule to run it, rather than the whole student object. – Jim Weaver Oct 31 '17 at 15:54
0

You said:

In the real world, the University set the rule as stated above and it's responsibility is to check if International students have documentation.

This means that the verification responsibility lies on University (and can be different for each University) and not the Student. All student can do is provide necessary information for University to verify.

The add method should get documents from retrieveDocuments and run through its rules to determine is student is allowed to be accepted.

tsolakp
  • 5,858
  • 1
  • 22
  • 28
  • Is my add method adequate given my requirements? –  Oct 30 '17 at 22:43
  • No. It needs to be refactored to contain both 'isVerified' and `retrieveStatus` logic in it. Student should not have `isVerified` because that method is University specific. – tsolakp Oct 30 '17 at 22:47
  • If `isVerified` is a university method(i understand why), this means I have to ask Student for information, do I do this via the getters or another approach? If I use the getters does this break encapsulation? –  Oct 30 '17 at 22:49
  • It depends what kind of information, but generally it wont break the encapsulation. For example in order to check if student is foreign you can either have 'getCountryOfResidence" method on student which University in USA can check if it is USA or have separate StudentRegistry that given a student will return the country of resident for that student. – tsolakp Oct 30 '17 at 22:53
  • I like the `getCountryofResidence()` method, but when you say it depends, what do you mean? Obviously the university will need to be aware of the Student in the fullest sense(id, name, status, documents). I'm providing getters for all my private variables, however, it make sense given what I'm doing, doesn't it? –  Oct 30 '17 at 22:56
  • I meant that you have two choices either `getCountryofResidence` or `StudentRegistry`. You only need to provide getters for information needed by University to make the validation decision. You can even just have `id` on student and based on that `id` your University class can lookup any information it needs to have. For example lookup Students address and payments from database or any other type of registry/repository. – tsolakp Oct 30 '17 at 23:00
  • I rather the `getCountryofResident()` because the classes are meant for a student who hasn't registered yet, but is in the process of, so they won't exist in the database to be looked up by `id` yet. –  Oct 30 '17 at 23:05
  • I should point out, that in the other thread the `getter` approach was considered a bad idea. –  Oct 30 '17 at 23:16
  • 1
    This is all about practicality and theory. Purists might say that `getters` are bad but without `getters` not much can be done to design usale application. It is all about balance and keeping responsibility where it belongs. – tsolakp Oct 30 '17 at 23:24