swaggest / php-json-schema

High definition PHP structures with JSON-schema based validation
MIT License
446 stars 50 forks source link

GOTO operator in PHP, really? #142

Closed christian-weiss closed 2 years ago

christian-weiss commented 2 years ago

https://github.com/swaggest/php-json-schema/blob/0dbf9e7857d065bde50605972933d6c38dbe6b8c/src/Schema.php#L1186

I just spotted this in your code base on the very first look, while i was evaluating if i should use your lib...

vearutop commented 2 years ago

I'm not sure I understand the issue. Do you mind elaborating it a bit?

christian-weiss commented 2 years ago

This image is from the official PHP manual: [alt_text] ...i think a "goto" operator is a code smell, so it indicates that the architecture of this code base is suboptimal designed.

Please consider to split that method (X):

  1. move first part of X into a separate method (A)
  2. move second part of X into another separate method (B)
  3. call A and B from X (method X is now very small)
  4. replace "goto" in A by a "return" statement

Use class members to pass data around if necessary (increase coherence of that class). This small refactoring is just following the boy scout rule - there is more to be done...

I recommend to read the book "Clean Code".

vearutop commented 2 years ago

I have the opposite opinion, thank you.