0

Person Class:

public class Person {


    private String firstName;
    private String lastName;

    public Person(String firstname,String lastname) throws InvalidDataException
    {
        setFirstname( firstname);
        setLastname(lastname);
    }


    public void personFirstName(String firstName) throws InvalidDataException { 
        setFirstname(firstName);
    }

    public void personLastName(String lastname) throws InvalidDataException {

        setLastname(lastname);
    }

    public String getFirstName() {
        return firstName;
    }

    public String getlasttName()
    {
        return lastName;
    }

    protected final void setFirstname(String firstname) throws InvalidDataException{
         if( firstname == null ||firstname.length() < 1) {
                throw new InvalidDataException("First Name Cannot be Empty");
         }
          this.firstName=firstname; 

    }

    protected final void setLastname(String lastname) throws InvalidDataException {

         if( lastname == null ||lastname.length() < 1) {
                throw new InvalidDataException("Last Name Cannot be Empty");

         }

         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
        setProfessorID(professorID);
    }

    public void setID(String professorID) throws InvalidDataException{

        setProfessorID(professorID);
    }

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

    private void setProfessorID(String ID) throws InvalidDataException{
         if( ID == null ||ID.length() < 1) {
                throw new InvalidDataException("ID Cannot be Empty");
         }
          this.professorID=ID; 

    }

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

}

I've done some research on implementing setters and calling them in my sub-class. By declaring them protected and final, I prevent the sub-class from overriding it and doing unwanted behavior. My question is this, can I now get rid of personFirstName() and personLastName()? My constructor doesn't use it, and they call the protected final setFirstname, and setLastname(). Would getting rid of the public setters cause an issue later on in development?

betteroutthanin
  • 7,148
  • 8
  • 29
  • 48
  • @T.J.Crowder - Would you suggest removing setFirstName()? My question is, would it pose a problem later on if I do? –  Jan 04 '15 at 18:07
  • 1
    @T.J.Crowder - I've read elsewhere that it's not desirable to do so. Because I was calling the setters in my constructors, and I read it's not recommended to call constructors with methods that can be overridden. –  Jan 04 '15 at 18:10
  • Ah, okay, so the point is to avoid calling potentially-overridden methods from the constructor. I can see that. :-) – T.J. Crowder Jan 04 '15 at 18:16

2 Answers2

1

You've said your goal is to avoid calling methods in the constructor that may be overridden by a subclass. If so, I'd approach it like this:

public Person(String firstName, String lastName) {
    this.privateSetFirstName(firstName);
    this.privateSetLastName(lastName);
}
private void privateSetFirstName(String firstName) {
    // ...your logic for setting the field...
}
private void privateSetLastName(String lastName) {
    // ...your logic for setting the field...
}
public void setFirstName(String firstName) {
    this.privateSetFirstName(firstName);
}
public void setLastName(String lastName) {
    this.privateSetLastName(lastName);
}

E.g., make the standard setters standard, and keep your implementation-specific details in private methods within your implementation.

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • Thanks. I was following the advice given here. http://stackoverflow.com/questions/12410338/most-appropriate-place-for-bounds-checking-constructor-or-setter/12410420#12410420. –  Jan 04 '15 at 18:17
  • @CharlesWhitfield: Yeah, as soon as you said, I got it. :-) – T.J. Crowder Jan 04 '15 at 18:18
  • @ T.J. Crowder - If I wanted to share the private methods with my subclass, I could declare them protected and final, correct (Like I did in my example above)? –  Jan 04 '15 at 18:22
  • @CharlesWhitfield: Sure, but my instinct would be not to. They're implementation detail within your class. The only correct way for a subclass to set those fields should be through the base constructor and `setFirstName`/`setLastName`. – T.J. Crowder Jan 04 '15 at 18:26
  • Much appreciated. One more question, the way I implemented it my example, it wouldn't be the desired way of doing it? or it's just too messy? –  Jan 04 '15 at 18:28
  • @CharlesWhitfield: The only real change with the above is that the standard setters remain standard setters. – T.J. Crowder Jan 04 '15 at 18:32
0

You should try to avoid doing stuff within the getters and setter, it can give you a hard time tracking down errors - check the data elsewhere. I think this much simpler version will do:

public class Person {

    private String firstName;
    private String lastName;

    public Person() {} // introducing an empty constructor gives you more flexibility


    public Person(String firstname, String lastname) {
        setFirstname( firstname);
        setLastname(lastname);
    }

    public String getFirstName() {
        return firstName;
    } 

    public String getLasttName() {
        return lastName;
    }

    public void setFirstName(String firstName) {
        this.firstName = firstName;
    }

    public void setLastName(String lastName) {
        this.lastName = lastName;
    }
}

public class Professor extends Person {

    private String  professorID;

    public Professor (super();) {} // introducing an empty constructor gives you more flexibility

    public Professor (String firstname, String lastname, String professorID) {
        super(firstname, lastname);
        this.professorID = professorID;
    }

    public void setID(String professorID) {
        setProfessorID(professorID);
    }

    public String getID() {
        return this.professorID;
    }
}
Journeycorner
  • 2,474
  • 3
  • 19
  • 43
  • I'm accustomed to doing it the way you've shown. However, when I posted my code other times before I was instructed not to write it that way. Also, implementing an no argument constructor means impartial objects can be created. –  Jan 04 '15 at 18:13
  • *"You should try to avoid doing stuff within the getters and setter..."* Basic checks in setters are perfectly normal and appropriate. – T.J. Crowder Jan 04 '15 at 18:15
  • In some cases, like in a hibernate entity, having an empty constructor is even mandatory: http://stackoverflow.com/questions/2935826/why-does-hibernate-require-no-argument-constructor . – Journeycorner Jan 04 '15 at 18:16
  • @Journeycorner: Whether to implement a zero-args constructor is entirely a case-by-case basis. Yes, you need them for certain entity frameworks. That doesn't mean you should always write them. Objects with state should never have invalid state; if args in the constructor are necessary to prevent invalid state, then args in the constructor are necessary. If a framework requires you to have potentially-invalid objects by requiring zero-args constructors, you just bow to the implementation necessity and contain the damage as best you can. – T.J. Crowder Jan 04 '15 at 18:20
  • @T.J.Crowder: I agree, we don't know the context so there is no silver bullet. However it looks like his classes represent some kind of model, and I would be really surprised to encounter a model that throws an exception after calling a getter. – Journeycorner Jan 04 '15 at 18:28
  • @Journeycorner: I don't see any exceptions thrown in getters in the OP's code. – T.J. Crowder Jan 04 '15 at 18:28
  • @T.J.Crowder: Sorry, I meant setter: " public void setID(String professorID) throws InvalidDataException{". Yes, it's not useless but not very common either. – Journeycorner Jan 04 '15 at 18:40
  • @Journeycorner: Exceptions from setters are not uncommon. – T.J. Crowder Jan 04 '15 at 18:42