Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can we have reasons or explanation on the best practices? #35

Open
AhSem opened this issue May 10, 2019 · 5 comments
Open

Can we have reasons or explanation on the best practices? #35

AhSem opened this issue May 10, 2019 · 5 comments

Comments

@AhSem
Copy link

AhSem commented May 10, 2019

No description provided.

@Konafets
Copy link

Do you have any particular best practice in mind you need an explanation for?

@AhSem
Copy link
Author

AhSem commented May 10, 2019

Most of them. I think my real question is: what are the standards to be considered as "best practices"? I mean, everyone has his/her own way and style of writing codes. What makes some of the styles be "best practices"?

Example: Single responsibility principle.
Currently, I am writing with the "bad" practice. I don't see how the separating the logic into several small functions is a better way. For me, it seems like there are more functions scattered around and makes longer codes, which could be hard to maintain in future.

  • I am trying to understand the benefits of each best practices shared here, and slowly to change my code to use the best practice.

Thank you.

@poncianodiego
Copy link

Please read about Solid principles and also the Laravel documentation to better understand why this best practices are in place. Laracasts has a great lesson about solid.

@Konafets
Copy link

Konafets commented May 11, 2019

The best practices should help you to reduce the in-mind-programming and make your code so clear, that you and others can read it as is would be englisch language.

Taking your example of SRP:
The bad variant is how programming is taught. You see this style in most of the 101 programming tutorials (offline and online) and in university where beginners get introduced into the concepts of programming. And this is fine because at the beginning you have to learn so much concepts at once, so that its not important or overwhelming to think in this direction. Your code works and you are happy :-)


You have one method and with all the stuff inside what should happen.

public function getFullNameAttribute()
{
    if (auth()->user() && auth()->user()->hasRole('client') && auth()->user()->isVerified()) {
        return 'Mr. ' . $this->first_name . ' ' . $this->middle_name . ' ' . $this->last_name;
    } else {
        return $this->first_name[0] . '. ' . $this->last_name;
    }
}

Lets break it down:

if (auth()->user() && auth()->user()->hasRole('client') && auth()->user()->isVerified()) {

To understand this if statement you need a deep understanding of Laravel because you have to know that there is an auth() method, a method to get the role and one to figure out if the user is verified.

return 'Mr. ' . $this->first_name . ' ' . $this->middle_name . ' ' . $this->last_name;

In the first case of the if statement, the name is appended with Mr. (which is a bit weird, because a user could also be female) and the middlename is included.

return $this->first_name[0] . '. ' . $this->last_name;

In the 2nd case you have to know that $this->first_name is an array and the firstname is at index 0, (which seems weird to me as I am reading this).

So a lot of in-depth knowledge is necessary to grasp this little method which does nothing else than outputting a different user name depending on some internal states of the user.

Lets have a look of the good example at the same method:

public function getFullNameAttribute()
{
    return $this->isVerifiedClient() ? $this->getFullNameLong() : $this->getFullNameShort();
}
  • its only one line -> less to read
  • its clear to any programmer what this code does without having much knowledge of the implementation details of Laravel
  • even the programmer who wrote that code can benefit:
    • he doesn't need to know at that point what the fullname or the shortname is. These are some details defined elsewhere.

Imagine you need a verified client multiple times in your project. With the bad example you have to write the same stuff over and over again, while with the good example you have it defined in one place and if you want to change it later you have to do it only once.


And don't get worried about too much methods in your class. There is no limit of methods a programming language or computer can handle, but a limit of lines per method a human can handle.

My methods are mostly 5-10 lines long. If they grow longer it is a sign to extract something from it into a dedicated method.

@N69S
Copy link

N69S commented Jul 18, 2019

@Konafets I dont see how using the auth() helper in a model can be a best practice. it's a hidden dependency.
I mean, session level should not interfere in the model level. it's a logic to be put in the controller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants