Eating ElePHPants

Idealism is a guide, not a rule

Incremental progress is progress

User a Service Locator

Larry, you so funny...

But Service Locators are eeeeevil!

Service Locator = passed DI Container

That's it

Remember what we said about small steps?

class Drupal {
  protected static $container;

  public static function setContainer(ContainerInterface $container = NULL) {
    static::$container = $container;
  }

  public static function getContainer() {
    return static::$container;
  }

  public static function service($id) {
    return static::$container->get($id);
  }

  public static function entityManager() {
    return static::$container->get('entity.manager');
  }

  // ...
}

function menu_menu_update(Menu $menu) {
  menu_cache_clear_all();
  // Invalidate the block cache to update menu-based derivatives.
  if (\Drupal::moduleHandler()->moduleExists('block')) {
    \Drupal::service('plugin.manager.block')->clearCachedDefinitions();
  }
}

class BookManager {

  public function bookTreeAllData($bid, $link = NULL, $max_depth = NULL) {
    $language_interface = \Drupal::languageManager()->getCurrentLanguage();
    // ...
    $tree[$cid] = $this->bookTreeBuild($bid, $tree_parameters);

    return $tree[$cid];
  }
}

class BookManager {

  public function bookTreeAllData($bid, $link = NULL, $max_depth = NULL) {
    $language_interface = $this->languageManager()->getCurrentLanguage();
    // ...
    $tree[$cid] = $this->bookTreeBuild($bid, $tree_parameters);

    return $tree[$cid];
  }

  public function languageManager() {
    return \Drupal::languageManager();
  }
}

class BookManager {

  public function bookTreeAllData($bid, $link = NULL, $max_depth = NULL) {
    $language_interface = $this->languageManager()->getCurrentLanguage();
    // ...
    $tree[$cid] = $this->bookTreeBuild($bid, $tree_parameters);

    return $tree[$cid];
  }

  public function languageManager() {
    return $this->languageManager;
  }
}

Service locators are a stepping stone
toward Dependency Injection

Break things in steps

Big refactoring breaks APIs

Deal

How to deal

BC shims are your friend
Deprecate immediately
Convert BC shims

Drupal 7

function cache_get($cid, $bin = 'cache') {
  return _cache_get_object($bin)->get($cid);
}

function cache_get_multiple(array &$cids, $bin = 'cache') {
  return _cache_get_object($bin)->getMultiple($cids);
}

function cache_set($cid, $data, $bin = 'cache', $expire = CACHE_PERMANENT) {
  return _cache_get_object($bin)->set($cid, $data, $expire);
}

Drupal 8 (early)

/**
 * @deprecated
 */
function cache_get($cid, $bin) {
  return cache($bin)->get($cid);
}

function cache($bin = 'cache') {
  return \Drupal::cache($bin);
}

function something() {
  $item = cache_get('menu', 'a_key');
}

function something() {
  $item = cache()->get('a_key');
}

function something() {
  $item = \Drupal::cache('menu')->get('a_key');
}

Nothing is more permanent than a temporary solution

This takes commitment to address

Temporary solutions

@deprecated (Never add uses)
Change notices (and Documentation!)
Peer review
E_USER_DEPRECATED?
Sunset plan: release blocking?

Changesets that just remove legacy use
must be acceptable

Improving code without adding features must be acceptable

Don't bite off more than you can chew

Session handling

2 years on 1 patch... fail :-(
Rearrange deck chairs: 2 in already.
We'll get there...

Procedural code inside class keyword is still progress

Form Builder

class FormBuilder {
  public function __construct(ModuleHandlerInterface $module_handler, KeyValueExpirableFactoryInterface $key_value_expirable_factory, EventDispatcherInterface $event_dispatcher, UrlGeneratorInterface $url_generator, TranslationInterface $translation_manager, CsrfTokenGenerator $csrf_token = NULL, HttpKernel $http_kernel = NULL) {
    $this->moduleHandler = $module_handler;
    $this->keyValueExpirableFactory = $key_value_expirable_factory;
    $this->eventDispatcher = $event_dispatcher;
    $this->urlGenerator = $url_generator;
    $this->translationManager = $translation_manager;
    $this->csrfToken = $csrf_token;
    $this->httpKernel = $http_kernel;
  }

  // ...

  public function getFormId($form_arg, &$form_state) {
    // 20 lines of fugly here.
  }

  public function buildForm($form_id, array &$form_state) {
    // 100 lines of code/comments here
    return $form;
  }
}

form.inc

function drupal_get_form($form_arg) {
  return call_user_func_array(
    [\Drupal::formBuilder(), 'getForm'],
    func_get_args()
  );
}

function drupal_build_form($form_id, &$form_state) {
  return \Drupal::formBuilder()->buildForm($form_id, $form_state);
}

function form_state_defaults() {
  return \Drupal::formBuilder()->getFormStateDefaults();
}

function drupal_rebuild_form($form_id, &$form_state, $old = NULL) {
  return \Drupal::formBuilder()->rebuildForm($form_id, $form_state, $old);
}

No program will ever be perfect, just make it better

Contain the crap

Form Builder

class FormBuilder {
  public function __construct(ModuleHandlerInterface $module_handler, KeyValueExpirableFactoryInterface $key_value_expirable_factory, EventDispatcherInterface $event_dispatcher, UrlGeneratorInterface $url_generator, TranslationInterface $translation_manager, CsrfTokenGenerator $csrf_token = NULL, HttpKernel $http_kernel = NULL) {
    $this->moduleHandler = $module_handler;
    $this->keyValueExpirableFactory = $key_value_expirable_factory;
    $this->eventDispatcher = $event_dispatcher;
    $this->urlGenerator = $url_generator;
    $this->translationManager = $translation_manager;
    $this->csrfToken = $csrf_token;
    $this->httpKernel = $http_kernel;
  }
}

The code around it is fine

Know when to push for better, and when it's good enough

Interfaces FTW

Refactor the fugly to be smaller

Eating ElePHPants

@Crell

What we want

Legacy code

How does one eat an elePHPant?

One bite at a time...

... with friends

... with lots of friends

WSCCI

Proudly Invented Elsewhere

The first taste

The big bite

Bottom up

Pros

Cons

Top down

Pros

Cons

Sprints

Configuration system

Bikeshed stoppers

Say WAT?

Write integration tests

That's the point

For new functionality

For existing functionality

drupal_http_request()

Use Guzzle instead

However!

Share with yourself

Performance

Make code reviews easy

Larry, you so funny...

How to deal

Drupal 7

Drupal 8 (early)

Temporary solutions

Session handling

Form Builder

form.inc

Form Builder

This is an ongoing process

One bite at a time, with friends

Larry Garfield

`drupal_http_request()`