Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Fantastic Bugs and How to Avoid Them

Fantastic Bugs and How to Avoid Them

Legacy code can be riddled with bugs — both ordinary and extraordinary. Quickly finding and conquering them is crucial for upgrades and for less painful maintenance. Join me as I demonstrate fixes and avoidance strategies for some of the most prevalent bugs found in legacy code.

Anna Filina

October 15, 2021

More Decks by Anna Filina

Other Decks in Programming


  1. Anna Filina • Coding since 1997 (VB4) • PHP since

    2003 • Legacy archaeology • Test automation • Public speaking • Mentorship • YouTube videos
  2. • Real-world "silly" bugs. • How they came to be.

    • How you can avoid making mistakes. • Also, I need to rant about bad code. What is this talk about?
  3. $paths = $this->getPaths(); $urls = array_map(function ($path) { return self::BASE_URI

    . $path; }, $paths); array_map(): Expected parameter 2 to be an array, string given
  4. private function getPaths(): array { return [ $this->path1, $this->path2, ];

    } We get earlier warnings, and IDEs can highlight problems in your code.
  5. private $path1 = ['list_products' => '/products']; private $path2 = ['view_cart'

    => '/cart']; We assumed that the paths were strings.
  6. $urls = array_map(function (string $path) { return self::BASE_URI . $path;

    }, $paths); Useful if closure is very long, but the warning is still not early enough.
  7. /** * @return array<int, string> */ private function getPaths(): array

    Type hints to describe array in more detail, then use a static analysis tool.
  8. /** * @return array<int, string> */ private function getPaths(): array

    { return [ $this->path1, $this->path2, ]; } ERROR: MixedReturnTypeCoercion - src/TypeMismatch.php:65:16 - The type 'array{0: mixed, 1: mixed}' is more general than the declared return type 'array<int, string>' ...
  9. private $path1 = '/products'; private $path2 = '/cart'; It's possible

    for these values to be set to something other than strings.
  10. /** * @var string */ private string $path1 = '/products';

    In older PHP versions, we use annotations. In PHP 7.4+, we can specify the type.
  11. /** * @return array<int, string> */ private function getPaths(): array

    { return [ $this->path1, $this->path2, ]; } Annotations potentially lie, unless you can prove them with static analysis.
  12. if ($path === '') { throw new InvalidArgumentException('Is blank'); }

    The problem with this is you have to remember to do it everywhere.
  13. $urls = array_map(function (string $path) { return self::BASE_URI . $path;

    }, $paths); It would be nice if instead of expecting a simple string, we could say that we expect something that is not blank.
  14. $urls = array_map(function (Path $path) { return self::BASE_URI . $path;

    }, $paths); We can declare a custom type and just assume that it's valid.
  15. final class Path { public function __construct( private string $path

    ) { Assert::that($path) ->notBlank(); } public function getValue(): string { return $this->path; } } Class is final; value comes through constructor and gets validated; class has no setters.
  16. /** * @return array<int, Path> */ private function getPaths(): array

    { return [ new Path('/products'), new Path('/cart'), ]; } We now swap our strings for Path objects and update the type hint.
  17. public function setPrice(int $price) { $this->price = $price; } $this->setPrice(1.15);

    1.15 float will be coerced to int, rounding it down to 1 without any warnings.
  18. <?php declare(strict_types=1); TypeError : Argument 1 passed to MyClass::setPrice() must

    be of the type int, float given declare_strict works per file, so you can progressively modernize your app.
  19. private string $stringPath; private Path $voPath; Since PHP 7.4, you

    can have property types, which will also prevent you from accessing a value that wasn't yet assigned.
  20. class Product { public $name; } //... $this->findByName($this->product->name); TypeError :

    Argument 1 passed to MyClass::findByName() must be of the type string, null given
  21. class ProductEntity { public $name; public $price; } final class

    Product { private string $name; private int $price; //... } ORM models/entities can be converted to stricter objects before you let your app interact with them.
  22. return new ProductName( $product->name ); We can even have smaller

    objects to represent a subset of fields, or even just one field.
  23. if ($product->getLastPrice() !== null) { return number_format($product->getLastPrice()); } TypeError :

    number_format() expects parameter 1 to be float, null given We just checked that it was not null, yet it still crashes due to a null.
  24. public function getLastPrice() { return array_pop($this->prices); } There is no

    guarantee that a method will return the same value each time.
  25. $lastPrice = $product->getLastPrice(); if ($lastPrice !== null) { return number_format($lastPrice);

    } Don't assume anything and assign the value to a local variable first.
  26. @$array[$foo->a()]; public function a() { trigger_error('my error', E_USER_ERROR); } $array[$foo->a()]

    ?? 'something else'; In an attempt to suppress missing index notices, we can suppress bigger problems. Avoid @.
  27. interface ApiAware { public function setApi(Api $api); } if ($class

    instanceof ApiAware) { $class->setApi($api); } *Aware interfaces are a common way to inject services.
  28. final class MyClass implements ApiAware { private $api; public function

    setApi(Api $api): void { $this->api = $api; } public function sendApiRequest() { $product = new Product(); $this->api->sendRequest($product); } } If setApi was not called before sendRequest for any reason, this will crash.
  29. final class MyClass { public function __construct( private Api $api

    ) {} public function sendApiRequest() { $product = new Product(); $this->api->sendRequest($product); } } Ditch those interfaces and use proper dependency injection.
  30. if (!empty($array)) { return $array[0]; } Trying to access array

    offset on value of type bool empty doesn't just operate on arrays.
  31. !== "" !== 0 !== 0.0 !== null === true

    !== [] Check specifically for what you need instead. Otherwise, types can get coerced and break the logic.
  32. PaymentGatewayInterface StripeGateway capturePayment PaypalGateway capturePayment preauthorizePayment capturePayment This happens when

    you implements additional methods, so it but only in some classes. Try to stick to the interface methods in your implementations.