In this method, I need to perform logic involving the ID, name, GPA, and major. So, those are the ones required. All the other student getters don't have to be populated. Is it ok to just first validate the Student object and then those four methods I need? I feel like this is a bit misleading because I am passing this Student object to this method, yet not all fields are required, so its really a half-populated object being sent to this method. Just seems strange to me.

+2 A:

If some of the properties must be populated always for a Student to be valid, you should consider defining a nondefault constructor with the required parameters, and removing any default constructors from the class (and if needed, validating the property values within the getters). This ensures that only valid Student objects can be created.

If the other properties are really optional for Students, it looks perfectly OK to me. Of course, you need to think through the use cases and analyze the domain model carefully, in order to decide which parameters are required and which are optional.

The scenario here is really the second paragraph in your response. The ones listed above in the report method do not HAVE to be set for a student to be valid.

2010-07-28 16:32:53

@smayers81 - one of the big advantages of objects vs. structs is the ability to guarantee that the object is always in a valid state. However, I can imagine a new Student that hasn't declared a Major and has no GPA because they've taken no classes yet. Good defaults, though, may be "Undeclared" and 0.0 so the object is still valid. All constructors for Student should require ID and Name, with additional constructors to provide Major and/or GPA.

The fact that it's called `printStudentReport` gives us a hint that it _should_ take a student. Even if it uses a given set of values _today_ doesn't mean that it can't change to use _more_ values _tomorrow_.

I am not quite sure this is _always_ beneficial. In fact, it would tie the caller(s) more tightly to the actual representation of both `Student` and `printStudentReport()`. And if you can't reuse `printStudentReport()`, there is not much benefit to balance this out. Not to mention the code bloat and duplication if the method is called from many different places...

Half-populated objects are quite common. Especially if you don't have control of the data source populating your objects. I say it would be just fine to only validate the Student fields that are required for printStudentReport(). I often write similar report-generating methods that validate based on the data that is necessary but will provide any extra data from the object if it is present.

One question that occurs to me is whether the logic really is Report logic or Student logic. In report you could code:

thing = (student.getX() + student.getY() ) * student.getZ();

or just

thing = student.getThing();

My take is that probably some stuff belongs in student.

So then we get the case, that we can't compute thing because some of X, Y or Z are not correctly initialised. So calling getThing() might throw an exception, but that feels weird. Why should an object offer some getThing() capability but then not be able to do it?

It seems to me that your Student class is in need of refactoring. There's a core set of capabilities that Students can do and they're sufficient to allow certain reports to be produced. Hence you've got say IRegistered interface and a richer IActiveStudent interface. Your report class needs an IRegistered and other classes need IActiveStudent.

Various Student objects change their capabilities in their lifetime, rather like a Caterpiller turns into a Moth. You don't have a fly() method on a caterpiller, and hence you don't need a canYouFlyYet() method on all your lepidoptra classes.

Totally agree. Its just that refactoring the Student class at this point in the game is really not feasible. Plus, there really isn't a good interface to use for this. This report is just an informational report printed ad hoc.

2010-07-28 16:53:07

A:

Think about the concept you're creating: a student report. It shouldn't matter that the method uses only a set of the student data, because those are the current requirements for your report. Maybe they'll change in the future. Maybe they won't. But it just seems like the right design, because it's more resilient to change.

Now the validation is trickier. Does the report need a special kind of validation, different than the normal validation for students? If that's the case, then by all means, validate it in the report:

But if the validation is common for a set of clients (maybe for printStudentReport and for saveStudentInDatabase), then you could create a validation class:

public class FloogleStudentValidator { // or some good name that tells us what this validation does
public void validate(Student student) { }
}
// ...
public void printStudentReport(Student student) {
new FloogleStudentValidator().validate(student);
// print the report....
}

You'd have different classes for the different types of student validation.

But, if the validation is common for the whole system, than I'd prefer to put it in the Student class itself, or validate it as it's populated in a student instance.