4
package cen.col.course.demo;

import java.io.Serializable;

public class Course implements Serializable {

private static final long serialVersionUID = 1L;
protected String code;
protected String title;
protected Professor professor;

public Course( String code) throws InvalidDataException {
    super();
    setCode(code);
}

public Course(String code, String title ) throws InvalidDataException  {
    this(code);
    setTitle(title);
}

public Course(String code, String title, Professor professor) throws InvalidDataException   {
    this(code,title);
    setProfessor(professor);
}
    public String getCode() {
    return code;
    }

protected void setCode(String code) throws InvalidDataException {
    if ( code == null || code.length() < 1) {
        throw new InvalidDataException("Course must have a course code");
    }
    this.code = code;
}

public String getTitle() {
    return title;
}

public void setTitle(String title)  throws InvalidDataException {
    if ( title == null || title.length() < 1) {
        throw new InvalidDataException("Course must have a title");
    }
    this.title = title;
}

public Professor getProfessor() {
    return professor;
}

public void setProfessor(Professor professor) {
    this.professor = professor;
}

public String toString() {
    String output = getCode() + ": [" + getTitle() + "]";
    if (getProfessor() != null ) {
        output += " is taught by " + getProfessor();
    }
    return output;
}

public boolean equals(Course c) {
    if ( ! this.getCode().equals(c.getCode())){
        return false;
    }
    if ( ! this.getTitle().equals(c.getTitle())){
        return false;
    } 
    // should the prof field be included in test for equality?
    if ( ! this.getProfessor().equals(c.getProfessor())){
        return false;
    } 
    return true;
}


}

I have Three Questions:

  1. I noticed my professor calling the setter methods from the constructors. I did a little searching around, and have mixed thoughts about it. Some say its okay, some say you have to be careful when your using subclasses, Is it okay to call your setters from the constructors?

  2. The constructors throw exceptions because she is calling the setters from the constructor. Now my question is, if calling the setters from the constructors isn't a safe way of doing it, What is the proper way of doing it? My guess would be to declare a no argument constructor, and build the object using setters.

  3. I guess doing this, is out of the question?

    Course createCourse = new Course("1234","Programming 1","Pam Halpert");

I am calling the constructor that takes 3 arguments, However, if calling the setter from the constructor is not safe, how would go about doing this, and have the exception in place? Could I use if statements? Check to see if something is blank and throw the exception if necessary?

Sylvia Rosemond
  • 197
  • 1
  • 3
  • 11
  • See [my answer to this question](http://stackoverflow.com/questions/12410338/most-appropriate-place-for-bounds-checking-constructor-or-setter/12410420#12410420). More likely than not, you want to make a method that can be called from both the constructor and the setter that will do validation and exception-throwing. This is necessary when designing objects that can be extended. – Brian Sep 21 '12 at 15:13
  • @Brian - Thank You very much. Your post helped me alot. Silly question, but How do I mark your response at the answer? – Sylvia Rosemond Sep 21 '12 at 15:17
  • for example: i have an answer below. on the left side, u ll see a tick image. just click on that:) thanks. – DarthVader Sep 21 '12 at 15:18

4 Answers4

1
  1. Calling the setters within the constructors generally has the advantage that sometimes setters already have some validation logic inside (like the setTitle in your example) and you don't want to duplicate this logic. However calling setters can lead to the problem, as you already mentioned, that subclasses may override them with unexpected behaviour. To solve this you can either make the setters private or final so that they can't get overriden. Calling only private/final setters is a good practice and should not lead to any problems.

  2. It is fine that a constructor getting invalid data throws an exception. You do not want to create an invalid object.

  3. It is rather bad practice to first create an empty object (through empty constructor) and then fill its data via setters. This way you will have for some time an object in a meaningless state which has some data filled, some data unfilled, and this might lead to troubles. Also, as another answer already mentioned you should think about reducing the numbers of constructors - is a Course without professor really valid? If not there doesn't need to be a constructor creating such an object...

daolaf
  • 167
  • 4
  • @daolaf- Thank for the help. My professor wrote this class and I just have a few questions about. But you are correct about the constructor without a professor isn't valid. – Sylvia Rosemond Sep 21 '12 at 15:31
  • @daolaf-Small question, in number 3 of your answer, how could I go about building the object, since its bad practice to have an empty object and build it using the setters. Would I build it by calling one of the constructors? – Sylvia Rosemond Sep 21 '12 at 15:39
  • Yes. Only implement the constructors which are building valid objects (filling all required data) and call them. – daolaf Sep 21 '12 at 15:51
0

Since this is homework, or some study, your professor prolly wanna show you things.

however ,
Course createCourse = new Course("1234","Programming 1","Pam Halpert");

is the best thing to do actually.

Depending on what you are developing, most of the time, you want to provide as little constructors as possible, unless you are designing a programming language. if you are working on a public API, or product, you should make sure that your consumers do not make mistakes, or abuse your API, if you allow them to create bugs.

Constructor can throw and exception, which is good.

As far as i see, calling the setter reason was doing some validation or some logic. which is fine.

Keep in mind, doing any work in constructor is considered a bad practice.

you should do it outside the class and pass them in as constructor arguments, or setter/getter.

DarthVader
  • 52,984
  • 76
  • 209
  • 300
  • @DarthVader-Thanks, In your opinion she was right to call the setters from the constructors, correct? – Sylvia Rosemond Sep 21 '12 at 15:23
  • yeah, that s legal. i think she wants to show a encapsulation. so you dont have direct access to the field. but u go through setters/getters. which is fine. – DarthVader Sep 21 '12 at 15:25
0

Personally I am not a huge fan of setters. I like immutability so in your example I would pass in the parameters to the ctor. Do the checking there and then assign to final fields. Setters would not exist. You would only have getters.

If you want to update then you can introduce copy constructors.

Then you would know that the object when it is constructed is in a state that is valid. If some parts of it are null then you can overload the constructor. You do not know which fields need to be populated by having no-arg constrcturs and setters. By enforcing it with parameters in the constructor you are forcing the object to be initialised in a valid state.

RNJ
  • 15,272
  • 18
  • 86
  • 131
  • `By enforcing it with parameters in the constructor you are forcing the object to be initialised in a valid state` which is good. no? – DarthVader Sep 21 '12 at 15:26
  • why no? @DarthVader. Sorry I dont understand your comment. Are you agreeing or disagreeing? :) – RNJ Sep 21 '12 at 15:30
  • i agree that `By enforcing it with parameters in the constructor you are forcing the object to be initialised in a valid state` this is the way it should be. – DarthVader Sep 21 '12 at 16:26
0

Calling setters is useful if your setters do some form of data validation. This allows you to place the validation in a single place and ensure that properties that are set at the point of instantiation comply with validation rules.

However, the problem with this is that subclasses could override these setters, meaning that your expected validation no longer occurs.

As such, it makes sense to create private setters that do the validation. Call these private setters in your constructors. If you want to have public setters as well, that's fine, just create a public wrapper setter around your private setter.


Side Note:

Your professor's example is a little off.

The validation seems designed to ensure that title and code are always set. However, the code also provides constructors that allow you to not set a title. This is almost certainly a bug (I would certainly flag it as one in a code review).

Dancrumb
  • 26,597
  • 10
  • 74
  • 130