0

I've created a Person class, and a class that inherits from it, the Professor class. Now, I've declared my setters private in the Person class and Professor class. I want the constructors to set the variables, by calling the setters and performing validation. Is what I've done correct? If not, what can I do to correct it?

Person Class:

public class Person {


private String firstName;
private String lastName;

public Person(String firstname,String lastname) throws InvalidDataException
{
    setFirstName(firstname);
    setLastName(lastname);
}

public String getFirstName() {
    return firstName;
}

private void setFirstName(String firstName) throws InvalidDataException {

 if ( firstName == null || firstName.length() < 1) {
      throw new InvalidDataException("Person Must have First Name");}

    this.firstName = firstName;
}

public String getLastName() {
    return lastName;
}

private void setLastName(String lastName) throws InvalidDataException {

     if ( lastName == null ||  lastName.length() < 1) {
          throw new InvalidDataException("Person Must have Last Name");}

    this.lastName = lastName;
}

Professor class

public class Professor extends Person {


private String  professorID;

public Professor(String professorID,String firstname, String lastname) throws InvalidDataException {
    super(firstname, lastname);
    // TODO Auto-generated constructor stub
    this.setID(professorID);

}

private void setID(String professorID) throws InvalidDataException{

     if ( professorID == null ||professorID.length() < 1) {
          throw new InvalidDataException("Person Must have ID");}
    this.professorID = professorID;
}

public String getID()
{
    return this.professorID;
}


public void printData()
{
     System.out.println("Professor ID: " + this.getID() + " First Name: " + this.getFirstName() + " Last Name: " + this.getLastName());
}

}

  • 3
    Why even have setter methods if you're going to declare them private? I'm greatly confused by your question, and mainly by your goal. – Hovercraft Full Of Eels Jan 03 '15 at 14:44
  • Take a look at both this links: [Why use getters and setters](http://stackoverflow.com/questions/1568091/why-use-getters-and-setter) and [Using setters in constructor](http://stackoverflow.com/questions/8501735/using-setter-methods-in-constructor-bad-practice), as those contain the answers for this question(s). – Iootu Jan 03 '15 at 14:46
  • @HovercraftFullOfEels - Won't I run into an issue in my subclass if I make it public? –  Jan 03 '15 at 14:48
  • 2
    Considering that your "getters" mainly check Strings for being neither null nor empty, you might have a static or utility method doing just that, and call it in the constructor (and/or public setters) before you assign to the class member. – laune Jan 03 '15 at 14:49

3 Answers3

2

Considering that your "setters" mainly check Strings for being neither null nor empty, you might have a static or utility method doing just that, and call it in the constructor (and/or public setters) before you assign to the class member.

public class Person {

    protected void check( String s, String msg ){
        if( s == null ||s.length < 1) {
            throw new InvalidDataException(msg);
        }
    }

    public Person(String firstname,String lastname) throws InvalidDataException{
        check( firstname, "Person's first name missing" );
        check( lastname, "Person's last name missing" );
        this.firstname = firstname;
        this.lastname = lastname;
    }

    public void setFirstname( String firstname ){
        check( firstname, "Person's first name missing" );
        this.firstname = firstname;
    }
}

But a bean shouldn't need guards like this. If there's a GUI, the GUI should do the validation, passing only correct values to object construction.

laune
  • 31,114
  • 3
  • 29
  • 42
  • 1
    It would work - but "incorrect" is an elastic term ;-) - That link from lootu explains what sound class design should heed. – laune Jan 03 '15 at 14:57
  • I like the way you've done this, so in my sub-class I can call check, because its protected, pass my variables, and error. Nice –  Jan 03 '15 at 15:00
  • As I mentioned: a utility class for checking values (not only String) might also be a good place. – laune Jan 03 '15 at 15:07
  • @CharlesWhitfield: indeed this is nice, 1+. Part of the issue of confusion is your calling your validation methods "setter" methods. The methods do not conform with the general use of this term and thus should not be called setters. – Hovercraft Full Of Eels Jan 03 '15 at 15:08
  • @laune - Could call the setter from the constructor, and let the setFirstName perform the check? –  Jan 03 '15 at 15:19
  • @laune-Do I need to declare it protected? Because in my subclass I'd call the base class and pass the arguments –  Jan 03 '15 at 15:27
  • You can make this public. – laune Jan 03 '15 at 15:28
  • @laune - Would it be bad practice to declare check private and let the constructor call it? –  Jan 03 '15 at 15:34
0

It is a bad practice to declare setters as private . because setters goal is to call them outside that class from the class instance. If you really want to fill your class properties with the constructor you may create a private functions that will build up your class. ** ps: if your class attributes are easy to fill you may fill them in your constructor .you dont need any support functions.

Yakov Mor
  • 21
  • 2
0

The best way would be to set the Person class members protected instead of private. Anyway, setters and getters are supposed to be public in OOD.

Upsilon42
  • 241
  • 2
  • 17