>> Erik Bernhardson <ebernhard...@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


_______________________________________________
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Reply via email to