Naming: getters, setters, methods


(Alexander Makarov) #1

Currently in Yii 3 codebase there are many ways of naming methods:

  1. getX() - explicit getters
  2. setX() - explicit setters
  3. isX() or shouldX() - getters for booleans
  4. withX() or withoutX() - immutable “setters”
  5. Action-methods such as EmitterInterface::emit() or ErrorHandler::handleError()
  6. Not explicit setters such as Route::host() or Route::to()
  7. Not explicit getters such as MatchingResult::methods().

What do you think about all these? Should we try to define a convention?


(Lubosdz) #2

Also hasX().
The question is a little unclear, though.
What’s wrong with Yii2 naming conventions ?
And how these can be improved in Yii3 ?


(Alexander Makarov) #3

There’s nothing wrong with Yii 2 conventions except that method naming is not part of these.

The questions are meant to be:

  1. Are you OK with such naming variety?
  2. Should we regulate that stricter? For example, all getters should have get, all setters should have set (if mutable) or with (if immutable).

#4

I don’t think we should force this convention everywhere. Some builders may be hurt by such strict convention ((new Query())->setFrom('table')?).


#5

(new Query())->from('table')->getOne() A setter should always return void. I would prefer for functions wtihout set prefix, that they always return the original objet itself.
Except for get*, has*, is*, to* (e.g. toString() )

Google a bit about this: found an interesting opinion, I am not sold on it, have to check it again after having slept over it.


edit: made the example code show how I wished it to be, instead of how it looks like when not.


(Insolita) #6

i agree, in some cases set/get prefix looks as overhead. and is/has/with etc looks better than get* prefixed like getIsAdmin() vs isAdimn().


#7

In this case it was used to provide virtual attribute - intended usage was $object->isAdmin.


(Insolita) #8

It just first example that coming in my head


(muitosefala) #9

My humble opinion:
I like the point 2, in the way code will be much more readable, and stricter means less code error possibilities, but I am not really sure.


(Stefano Mtangoo) #10

Here are my opinions

This should be used for strict setters that does nothing but setting/getting

These should reflect exactly what they say. For example isRunning() and shouldRun() should just do that, checking whether it is already running or we are allowed even to run at that point.

Same as above, me thinks

Am fine with name that reflects what it is doing

On Route is should be getHost() really host sounds like a command to host than a getter. So is to which is I don’t know what it does from reading it. Have to consult docs (which is bad. Mehods should try to be self docmenting)

methods should be getMethods()

That is my thoughts


(Alexander Makarov) #11

Route is currently used like the following:

$routes = [
    Route::get('/test/{id:\w+}')
        ->to(new ActionCaller(SiteController::class, 'testParameter', $container))
        ->name('test_id');
]

With stricter names it will become:

$routes = [
    Route::get('/test/{id:\w+}')
        ->withTo(new ActionCaller(SiteController::class, 'testParameter', $container))
        ->withName('test_id');
]

(Tomasz Kane) #12

Definitely current version looks better.


(Serban Cristian) #13

Except you can call if ($obj->isAdmin) on getters starting with get which is a time saver and shorter than if ($obj->isAdmin())

L.E: Nevermind, this has been discussed.


(Syakur Rahman) #14

I’m totally fine with the current way.

Enforcing standards, just for the sake of it, doesn’t seems a good enough reason for adjusting a perfectly good working system