Erik
Bernhardson <ebernhardson(a)wikimedia.org> wrote:
Moving forward, the Flow team
is considering using a php implementation
that follows the ideas of the haskell Maybe monad(
https://github.com/schmittjoh/php-option ). This is, in concept, rather
similar to the Status class the wikitext parser returns. We would like to
use this library as a way to more explicitly handle error situations and
reduce the occurrences of forgetting to check false/null. This particular
pattern is very common in Functional languages.
I don't think exceptions are evil, they are more structured gotos and
goto can be used properly to handle errors.
Status class has similar problem as unchecked exceptions - you never
know which exception might come, and similarly, you never know
what kind of Status you might inherit from the code called.
However, exceptions can form a hierarchy, which allows the
caller to react selectively to the class of problems we have.
I recently was struggling with this piece of MediaWiki
code, which interprets values of internalAttemptSave:
(from EditPage.php)
1195 // FIXME: once the interface for internalAttemptSave() is made nicer, this should
use the message in $status
1196 if ( $status->value == self::AS_SUCCESS_UPDATE || $status->value ==
self::AS_SUCCESS_NEW_ARTICLE ) {
1197 $this->didSave = true;
1198 if ( !$resultDetails['nullEdit'] ) {
1199 $this->setPostEditCookie();
1200 }
1201 }
1202
1203 switch ( $status->value ) {
1204 case self::AS_HOOK_ERROR_EXPECTED:
1205 case self::AS_CONTENT_TOO_BIG:
1206 case self::AS_ARTICLE_WAS_DELETED:
1207 case self::AS_CONFLICT_DETECTED:
1208 case self::AS_SUMMARY_NEEDED:
1209 case self::AS_TEXTBOX_EMPTY:
1210 case self::AS_MAX_ARTICLE_SIZE_EXCEEDED:
1211 case self::AS_END:
1212 return true;
1213
1214 case self::AS_HOOK_ERROR:
1215 return false;
1216
1217 case self::AS_PARSE_ERROR:
1218 $wgOut->addWikiText( '<div
class="error">' . $status->getWikiText() . '</div>' );
1219 return true;
1220
1221 case self::AS_SUCCESS_NEW_ARTICLE:
1222 $query = $resultDetails['redirect'] ? 'redirect=no'
: '';
1223 $anchor = isset( $resultDetails['sectionanchor'] ) ?
$resultDetails['sectionanchor'] : '';
1224 $wgOut->redirect( $this->mTitle->getFullURL( $query ) .
$anchor );
1225 return false;
1226
This code is somehow replicated in ApiEditPage.php, but in another way.
I wanted re-use this logic in the Collection extension
(which sometimes creates some page in bulk on behalf
of the user) and I really wished error reporting
was done with exceptions. At top level, they
could be handled in EditPage.php way, API would
return exception objects instead and other
extensions could selectively handle some values
and ignore others - for example, I would be happy
to get the standard EditPage.php behaviour
for most of the errors I am not particularly
interested in.
Regarding the use of php-option:
php-option seems to be as a handy way to provide substitute
objects in case of errors; I think this case
comes not very often and is arguably better
handled by the use of factory methods, i.e.
a method is reponsible to deliver foreign
instance of some class; factory methods can
be defined once for particular orElse/getorElse
situation; it is also easier to make sure
that particular instances will deliver
the same (or similar) interface.
And factory methods can be overriden
if some particular implementation
needs different creation of fallback objects.
Instead of having:
return $this->findSomeEntity()->getOrElse(new Entity());
One could have:
interface StuffNeededFromEntity {
/* what Entity() really provides */
}
class Entity implements StuffNeededFromEntity {
}
class FallbackEntity implements StuffNeededFromEntity {
}
/*...*/
/* In the code trying to findSomeEntity */
/* @return StuffNeededFromEntity */
protected function entityFactory() {
$entity = $this->findSomeEntity();
if (null === $entity) {
return new FallbackEntity();
}
}
/* replace return $this->findSomeEntity()->getOrElse(new Entity()); */
return $this->entityFactory();
[The use of interface/implements above is for clarity only,
one does not actually need to use those features]
I agree with php-option author that if ($x===null) boilerplate
should be avoided, but in my opinion the solution is
to do the logic once and abstract it properly for re-use
and not hide it in some syntatic sugar instead.
Imagine what happenes if we need some constructor
parameters for new Entity() -> instead of updating
entityFactory once one needs to go through all
those "orElse" cases again.
//Saper