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.

Transcript

1. Fantastic Bugs and
   How to Avoid Them
   LONGHORN PHP | OCT 2021 @afilina
   

   View Slide

2. Zend Framework 1 has undefined variables.
   

   View Slide

3. Anna Filina
   • Coding since 1997 (VB4)
   • PHP since 2003
   • Legacy archaeology
   • Test automation
   • Public speaking
   • Mentorship
   • YouTube videos
   

   View Slide

4. • 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?
   

   View Slide

5. Bugs as justification for
   following best practices.
   

   View Slide

6. $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
   

   View Slide

7. private function getPaths()
   {
   return '[
   {"list_products":"/products"},
   {"view_cart":"/cart"},
   ]';
   }
   You have to read every method to know
   what will happen.
   

   View Slide

8. private function getPaths(): array
   {
   return [
   $this->path1,
   $this->path2,
   ];
   }
   We get earlier warnings, and IDEs can
   highlight problems in your code.
   

   View Slide

9. $paths = $this->getPaths();
   $urls = array_map(function ($path) {
   return self::BASE_URI . $path;
   }, $paths);
   Array to string conversion
   

   View Slide

10. private $path1 = ['list_products' => '/products'];
    private $path2 = ['view_cart' => '/cart'];
    We assumed that the paths were
    strings.
    

    View Slide

11. private $path1 = '/products';
    private $path2 = '/cart';
    We change paths to
    strings.
    

    View Slide

12. $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.
    

    View Slide

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

    View Slide

14. composer require --dev vimeo/psalm
    vendor/bin/psalm --init
    Detected level 7 as a suitable initial default
    vendor/bin/psalm src
    

    View Slide

15. /**
    * @return array
    */
    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' ...
    

    View Slide

16. private $path1 = '/products';
    private $path2 = '/cart';
    It's possible for these values to be set to
    something other than strings.
    

    View Slide

17. /**
    * @var string
    */
    private string $path1 = '/products';
    In older PHP versions, we use annotations.
    In PHP 7.4+, we can specify the type.
    

    View Slide

18. /**
    * @return array
    */
    private function getPaths(): array
    {
    return [
    $this->path1,
    $this->path2,
    ];
    }
    Annotations potentially lie, unless you
    can prove them with static analysis.
    

    View Slide

19. if ($path === '') {
    throw new InvalidArgumentException('Is blank');
    }
    The problem with this is you have to
    remember to do it everywhere.
    

    View Slide

20. $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.
    

    View Slide

21. $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.
    

    View Slide

22. 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.
    

    View Slide

23. composer require beberlei/assert
    Look for assertion libraries to help you
    with the validation.
    

    View Slide

24. /**
    * @return array
    */
    private function getPaths(): array
    {
    return [
    new Path('/products'),
    new Path('/cart'),
    ];
    }
    We now swap our strings for Path
    objects and update the type hint.
    

    View Slide

25. array_map(function (Path $path) {
    return self::BASE_URI . $path->getValue();
    }, $paths);
    When we see a Path, we can
    assume that it's ever blank.
    

    View Slide

26. Float coercion.
    

    View Slide

27. 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.
    

    View Slide

28. 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.
    

    View Slide

29. Strict types.
    Strict types.
    Strict types.
    

    View Slide

30. 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.
    

    View Slide

31. NPEs everywhere.
    

    View Slide

32. class Product
    {
    public $name;
    }
    //...
    $this->findByName($this->product->name);
    TypeError : Argument 1 passed to MyClass::findByName() must be of
    the type string, null given
    

    View Slide

33. class Product
    {
    public string $name;
    }
    The solution is to simply
    declare the type.
    

    View Slide

34. 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.
    

    View Slide

35. return new ProductName(
    $product->name
    );
    We can even have smaller objects to represent a
    subset of fields, or even just one field.
    

    View Slide

36. 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.
    

    View Slide

37. public function getLastPrice()
    {
    return array_pop($this->prices);
    }
    There is no guarantee that a method will
    return the same value each time.
    

    View Slide

38. $lastPrice = $product->getLastPrice();
    if ($lastPrice !== null) {
    return number_format($lastPrice);
    }
    Don't assume anything and assign the
    value to a local variable first.
    

    View Slide

39. @$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 @.
    

    View Slide

40. interface ApiAware
    {
    public function setApi(Api $api);
    }
    if ($class instanceof ApiAware) {
    $class->setApi($api);
    }
    *Aware interfaces are a common way
    to inject services.
    

    View Slide

41. 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.
    

    View Slide

42. Error : Call to a member function sendRequest() on null
    

    View Slide

43. 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.
    

    View Slide

44. Dependency injection
    is your friend.
    

    View Slide

45. if (!empty($array)) {
    return $array[0];
    }
    Trying to access array offset on value of type bool
    empty doesn't just operate
    on arrays.
    

    View Slide

46. empty("");
    empty(0);
    empty(0.0);
    empty("0");
    empty(null);
    empty(false);
    empty(array());
    All these things are
    considered empty.
    

    View Slide

47. !== ""
    !== 0
    !== 0.0
    !== null
    === true
    !== []
    Check specifically for what you need instead.
    Otherwise, types can get coerced and break the logic.
    

    View Slide

48. 0.99 + 0.01 === 1
    This is false.
    

    View Slide

49. IEEE 754
    floating point arithmetic.
    In simple terms, if you use floats,
    numbers may not add up.
    

    View Slide

50. $amountInCents + 1
    Operate on integers when possible.
    Divide at the presentation layer.
    

    View Slide

51. Interfaces vs concrete classes.
    

    View Slide

52. /** @var PaymentGatewayInterface */
    $gateway = $this->getSelectedGateway();
    $gateway->preauthorizePayment();
    Even though the gateway is the right type, the
    method might not exist.
    

    View Slide

53. 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.
    

    View Slide

54. @afilina
    Questions?
    

    View Slide