CODEWITHSUNDEEP
oop
Daniel Calderon Mori
Daniel Calderon Mori

Reputation: 5776

Is it good design to create an object inside a getter?

I have a bean class, say, Teacher that has a status active or inactive and each of those states have an identifier. To implement this behaviour, my team decided to have a Status bean with both the id and name attributes.

Now, let's say I'm a client of a service that uses a Teacher bean. In order to use said service I would have to create the Teacher bean:

Teacher teacher = new Teacher();
teacher.setName("Some name");
...
Status status = new Status();
status.setId(1);
...
teacher.setStatus(status);

The question is: would It be good OOP design if we implement the Teacher.getStatus() as

Status status = null;

Status getStatus() {
    if (status != null) {
        return status;
    } else {
        return new Status();
    }
}

So I don't call the Status constructor anytime I want to create a Teacher bean. Like this:

Teacher teacher = new Teacher();
teacher.setName("Some name");
teacher.getStatus().setId(1);

Would that be considered a bad practice? If so, why?

Upvotes: 0

Views: 66

Answers (2)

LAL
LAL

Reputation: 508

I think it is a bad design to return a new in a function. It will bring you to memory leaks. Each time you'll call getStatus you will create a new Status without deleting it. You can easily have the same design like this: 

Teacher teacher = new Teacher();
teacher.setName("some name");
teacher.getStatus().setId(1);

public class Teacher{
    private Status myStatus = new Status();
    public void Teacher(){...}
    public Status getStatus(){return myStatus;}
}

public class Status{
    private int Id = 0;
    public void Status(){...}
    public void setId(int p_id){Id = p_id}
}

Upvotes: 1

user1618143
user1618143

Reputation: 1748

It's not bad practice, per se, but it seems unnecessary in this example. If the object you're writing a getter for has a lot of overhead on construction - if it's being pulled out of a database, for example - this general approach is quite reasonable. There is, however, one thing you'll want to fix. Instead of creating and returning a new Status object, set this.status=new Status() and return it. That way you won't create two new Status objects if you call getStatus() twice.

Upvotes: 2

Related Questions