server icon indicating copy to clipboard operation
server copied to clipboard

Refactor userـldap app commands

Open fsamapoor opened this issue 2 years ago • 2 comments

Summary

I have made some adjustments to the apps/user_ldap/lib/Command classes to improve the code readability.

The improvements in this PR include but are not limited to:

  • Using PHP8's constructor property promotion
  • Adding return types
  • Replacing return code integers with more readable strings.
  • Using early returns

Checklist

fsamapoor avatar Aug 17 '23 08:08 fsamapoor

Looks like some void-annotated methods do actually return values?

Hello @Fenn-CS, Thank you for reviewing the PR.

No method with the return type of void returns any values. The git diff seems like the return belongs to the method with a void return type, but it actually does not.

These two lines do not belong to the same method:

image

Here is the expanded diff:

image

fsamapoor avatar Aug 28 '23 06:08 fsamapoor

I’m not sure why there is so much hate towards else statements, I find things clearer with them that without. Especially the continue in the loop instead of a simple else looks convoluted. I like the strong typing, constructor promotion and constant usage in returns though.

Please revert at least the match (true).

Thank you for taking the time to review the changes Come. This was one of my older PRs and I appreciate your suggestions.

fsamapoor avatar Jan 24 '24 07:01 fsamapoor