Suggest to use isser / hasser naming of the class's methods if them return boolean value

557 views
Skip to first unread message

Victor Bocharsky

unread,
Feb 12, 2016, 3:07:05 AM2/12/16
to PHP Framework Interoperability Group

Hi there!


Could we add some standard for isser / hasser for boolean values or include it to already proposed one? I mean name of class method's like `isPublished()`, `isWritable()`, `hasHeader()` etc. Seems it's a generally accepted terminology, but I don't find any mention of it. However, accepted PSR-7 already use them.


Cheers!


Andreas Hennings (donquixote)

unread,
Feb 15, 2016, 5:34:01 AM2/15/16
to PHP Framework Interoperability Group
Generally I agree with this. But we need to be careful to cover all cases.

    grep -R --after-context=3 "   \* @return bool" . | grep "public function"

I did this in a quite big vendor directory (Drupal 8), and here are the patterns I find:
Obviously, there are some methods that return bool, but are not issers or setters, but simply report success or failure. We can ignore these.
Also, I don't think this should apply to static methods - hence the "public function" in the grep.

Otherwise, there is a lot of isAdjective(), and hasNoun(*).
But then there are methods that don't have an is* or has* prefix, but use dedicated verbs:
- exists()
- needsNormalization()
- supportsDecoding()
- areOpen() (symfony\Process\Pipes\PipesInterface::)
- requiresDoubleQuoting()
- commandShouldRun() (symfony\Console\Event\ConsoleCommandEvent::)
- canBeExtracted($file) (symfony\Translation\Extractor\AbstractFileExtractor::)
- initialized()
- contains($id)
- containsKey($key)
- accepts($expected, $actual)
- matches(..)

Maybe sometimes the verb is not the first part of the method name. The following example does not really count because it is a static method..
- methodExists()

I also found some that would be renamed with this new standard.
- valid() -> isValid()
- eof() -> ?

Richard Fussenegger

unread,
Feb 15, 2016, 4:21:55 PM2/15/16
to PHP Framework Interoperability Group
Dictating things like this with rules is usually a very bad idea because it limits people in being creative and creating good designs. Mentioning things like this during a code review where you see potential to improve some wording is not and you should do it there. In good OO we usually strive for things that result in nice sentences; I know that some disagree but this is common consens among best literature and I therefore simply assume for now that you agree. For instance going back to the exists() example from above, consider the following:

$file = new File('/some/path/to/a/file');

if ($file->exists() === false) {
   
throw new FileNotFoundException($file);
}

$stream
= $file->toStream();

while ($stream->eof() === false) {
   
if ($stream->readLine()->matches('/php-fig/')) {
       
break;
   
}
}

This is of course a toy example but it should illustrate that freedom in design is important in order to create nice interfaces.

Do not limit yourself by introducing strict rules that do not help anyone! Conventions should contain things that are actually helpful and not things that limit you in your creativity.

Andreas Hennings

unread,
Feb 15, 2016, 5:12:32 PM2/15/16
to php-fig
Possibly you are right, and rules for this don't really help.
But where does your example violate the proposed standard?

If we say that verb forms are ok, and non-verbs should be prefixed with is*() and has*().
exists() would be perfectly fine, because it is a verb.

Only the eof() would be a violation. It would become sth like isAtEndOfFile(), or isAtEof(), or isEof().
This added verbosity does not really help anyone, so I agree.

But:
I think to remember seeing some code with inconsistent naming of these boolean methods, that would have improved by switching to a pattern with is*() and has*() and verb*().
A lose recommendation published somewhere would allow code reviewers to justify their review with a link.
But maybe it is good enough if such a recommendation can be found in a blog post somewhere.

In the end, I don't really care that much..



--
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/Srcx49GugaQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/1d2e71d5-34bc-4c68-b92c-9c6732a85ff4%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

Richard Fussenegger

unread,
Feb 16, 2016, 12:18:01 PM2/16/16
to PHP Framework Interoperability Group
Applying your rules would work for me if it would be SHOULD all over the place and not MUST. I also think that it would be very valuable to have such a document for people while doing code reviews. However, I think that a PSR would be a better thing that a Bylaw and I think that it should clarify other naming recommendations and not only this one.

I do not like your solutions for eof() but I think there are many alternatives available:

$stream->ended();
$stream
->finished();
$stream
->depleted(); // I love this word, lol.
$stream
->concluded();
$stream
->hasMoreData();
$stream
->isComplete();
$stream
->completed();

I admit that I was startled too much by the initial post and feared that this could go in a direction where it dictates to prefix your stuff with is and has all the time without bringing value. I encountered such so called coding conventions too often. (System Hungarian Notation, anyone?)

Krzysiek Piasecki

unread,
Feb 17, 2016, 5:44:18 AM2/17/16
to PHP Framework Interoperability Group


W dniu piątek, 12 lutego 2016 09:07:05 UTC+1 użytkownik Victor Bocharsky napisał:

Hi there!

Could we add some standard for isser / hasser for boolean values or include it to already proposed one? I mean name of class method's like `isPublished()`, `isWritable()`, `hasHeader()` etc. Seems it's a generally accepted terminology, but I don't find any mention of it. However, accepted PSR-7 already use them.



Suppose my class extends ArrayObject. So I will be never compatible with a standard, because of offsetExists?


-- 
KP

Andreas Hennings

unread,
Feb 17, 2016, 6:12:18 AM2/17/16
to php-fig
I suppose this depends on the details of the "standard" (or "recommendation", as I would prefer).
Of course if you are dealing with native APIs, you always have an excuse.

You could rename it to hasOffset($offset).
But then how do we distinguish a method hasOffset($offset) from another method hasOffset() with no parameter?
The former would tell us if a specific offset exists, the latter if there is any offset at all.
The recommendation could be worded so that has*() and is*() only applies to parameterless methods, and only if there is not another, more meaningful, verb.

Besides, it should be mentioned that the object itself must be the active noun in the sentence. E.g. "$file->isWritable()" means "(The) file is writable". Or rather "Is the file writable?" - duh :)
Whereas $array->existsOffset()" is wrong, because the array is not the active noun, the offset is.

Overall, there are many cases where is*() and has*() are a good idea, but we should be really careful not to let this apply to the wrong cases. And maybe just saying "can be a good idea" could already be enough. (not necessarily in a PSR)


--
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/Srcx49GugaQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

Richard Fussenegger

unread,
Feb 17, 2016, 12:49:54 PM2/17/16
to PHP Framework Interoperability Group
That is exactly the reason why I said that such a PSR can only contain SHOULD and not MUST. It is not always possible to follow such rules and it makes no sense to follow them all the time.
Reply all
Reply to author
Forward
0 new messages