DEV Community

Discussion on: How you can reduce usage of getter methods in your code

Collapse
 
bgadrian profile image
Adrian B.G.

Noo please, do NOT reduce the number of getters, the alternatives are much worse.

I suggest reading the "Effective Java 3rd Edition" even if you code in PHP, C# or any OOP language, you will find there some counter examples and many real world big projects examples on most pitfalls.

The examples from this article are taken out of context and you do not see the negative outcomes on the long run.

Using toString() should NOT be trusted, it is used only for debuging purposes. If you make a public class and its users will start to use toString by any chance (giving them the false trust that toString() is a replacement for getFullName()). You just did a dependency and a contract that will be break anytime. toString() will be modified during iterations, but FullName() will always hive you the fullName.

Sorting, if you choose to sort by only one criteria (which is not often the case), you do not put it in a compareTo! You do not make decisions for the users of your Class, let them make sort by ANY field by providing getters.

belongsTo() is another big mistake. To find out which company the users belongs to you will have to call the function O(n) times (where N is the number of companies from your database). Should I say more?

A lot worse would be to make a public property instead of setters/getters.

Collapse
 
aleksikauppila profile image
Aleksi Kauppila

Sorting, if you choose to sort by only one criteria (which is not often the case), you do not put it in a compareTo!

You can design compareTo() any way you like. With a callback for instance. Here, i'm concerned about business logic and there's generally quite limited amount of ways certain entities need to be ordered. I don't want to mix presentation concerns to this issue.

You do not make decisions for the users of your Class, let them make sort by ANY field by providing getters.

What? This makes absolutely no sense. Who else makes the decisions?

belongsTo() is another big mistake.

Consider this:

class Customer
{
    private $shifts;
    // ...
    public function addShift(Shift $shift): void
    {
        if (!$shift->belongsTo($this)) {
            throw new Exception("Shift doesn't belong to customer");
        }
        $this->shifts->add($shift);
    }
}

class Shift
{
    private $customer;
    //...
    public function belongsTo(Customer $customer): bool
    {
        return $this->customer->equals($customer);
    }
}

// application code
$customer = $customerRepository->findByName("Big Corp");
$shift = new Shift($customer, new DateTime("2018-01-01 12:00"));

Shift doesn't have any reason to exist without a customer who orders an employee to do some work. Customer is an aggregate for a Shift. We deal with all persistance issues through Customer and that is why we add Shifts to Customer's collection. We also have to make sure that we don't add "Small Corp's" Shifts to "Big Corp's" collection and use belongsTo() to make sure that doesn't happen.

We decide to put focus on Customers in this application. Shifts and Customers are not equals in terms of importance. We also don't access Shifts directly from the database. This happens through Customers.

Maybe the focus shifts at some point and then we change the model but for now belongsTo() serves this use case well.

The big picture here is that we shouldn't have to know the internals of objects. We need to limit the public API as much as it makes sense. Otherwise further development becomes harder and slower as time goes by.

Collapse
 
bgadrian profile image
Adrian B.G.

The big picture here is that we shouldn't have to know the internals of objects. We need to limit the public API as much as it makes sense. Otherwise further development becomes harder and slower as time goes by.

Exactly, but you are also limiting your classes usages.
Without access to the $company you cannot do: group by company, sort by company, are in the same company, show the company in a table, change the company, should I continue?

If it is a property and it is not used, maybe it should not exists.

Thread Thread
 
aleksikauppila profile image
Aleksi Kauppila

I'd prefer calling this design. I design what interactions are appropriate for objects.

Employee has to know $company because when we save it to database (or anywhere) this information is important. To be fair i didn't communicate this that clearly.

Accessing the company of an Employee must come from a use case. Until we have use case we avoid exposing this information. When we have use cases these things may be able to catch a certain more natural meaning. So instead of just doing data processing based on employee attributes we could have named reports or something.

This discussion is very difficult when the model is as crude as my code example here.

Thread Thread
 
bgadrian profile image
Adrian B.G.

Maybe you even forgot to mention that you control your entire app (monolith), and is easy to refactor the classes AND maybe it is even a small app (and you can refactor ALL the usages of a class in a matter of minutes.

my problems with this article will happen when you do not have access to the source code of the classes and you are just an User. This includes libraries, APIs,RPCs, frameworks. Also this could happen when the code has millions of LOC.

Probably some of the readers will build these kind of classes too, and you cannot and should not think of all use cases when you are building an API/expose your classes. If you try to do that it will bloat your classes, give the users flexibility, let them decide how to use it, of course in limited reason.

Thread Thread
 
aleksikauppila profile image
Aleksi Kauppila

As implied in code examples, this is business layer code. Yes, i expect people working with applications to be able to change their code

When you're working with libraries, you usually don't have to worry about them containing entities such as Company or Employee. But defensive practices work well when designing a library too. If there's only one way to use a class, it's virtually impossible to break the code.

Collapse
 
zhu48 profile image
Zuodian Hu

I don't know PHP, but I do know, in order of most to least knowlesge, C++, Java, and C#.

Seems to me that setters and getter being required for your object to work well might be a symptom of objects that can be better designed. If you're working with employees, for example, the employee object should have logical methods that do logical things to them. For example, promote, or get a standard format legal name. When you sort and search through bunch of employees by some attribute, wouldn't it be better to work with a data structure object instead? This way, checking employee membership in some entity is an action on the data structure. Not to say this would be how everybody would design an employee object, it depends on the use case. The big picture is, if you need to work with a set of objects, maybe use a data structure object. If you manipulate single objects, probably more useful to put those methods in the single object's class.

Collapse
 
aleksikauppila profile image
Aleksi Kauppila

Some collections contain data structures, some contain objects.

Footballing example:

  • Competitition contains matches
    • Exposes Standings by calculating them from Match results
    • Cannot contain matches from other competitions
    • Matches are entities
    • Matches collection is hidden, but used internally to create Standings.

  • Match contains events (goals, bookings, substitutions)
    • Exposes MatchStatistics by calculating them from Event occurrences by home and away team
    • Cannot contain events from other matches
    • Events are data structures
    • Events collection is hidden, but used internally to create MatchStatistics
Thread Thread
 
zhu48 profile image
Zuodian Hu

Sorry, I'm not sure if you're trying to make a point or not?

Thread Thread
 
aleksikauppila profile image
Aleksi Kauppila

Yes :D there may be sorting happening inside an aggregate entity and that membership in a collection needs to be checked whether we are dealing with objects or data structures. However the objects in a collection usually only serve one aggregate, eg. Match / Events. That's why ordering algorithm can be coded into a compareTo -method of that object inside the collection. No need to order based on exposed attributes.

Thread Thread
 
zhu48 profile image
Zuodian Hu

Oh, OK. Yeah I agree with you; for modularity and stability, it's better to opaquely define how comparison should work than to expose data and let the user misuse and depend on that data. I think the big disagreement most people have with that idea is that encapsulation isn't always the most important thing in the world.