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

(Refactor) Code Quality Changes #1371

Merged
merged 3 commits into from
Jun 2, 2020
Merged

(Refactor) Code Quality Changes #1371

merged 3 commits into from
Jun 2, 2020

Conversation

HDVinnie
Copy link
Collaborator

@HDVinnie HDVinnie commented Jun 2, 2020

Refactors Performed In This PR Are As Follows:

Complete Dynamic Properties

Add missing dynamic properties

 class SomeClass
 {
+    /**
+     * @var int
+     */
+    public $value;
     public function set()
     {
         $this->value = 5;
     }
 }

Remove Sole Value Sprintf

Remove sprintf() wrapper if not needed

 class SomeClass
 {
     public function run()
     {
-        $value = sprintf('%s', 'hi');
+        $value = 'hi';

         $welcome = 'hello';
-        $value = sprintf('%s', $welcome);
+        $value = $welcome;
     }
 }

Remove Extra Parameters

Remove extra parameters

-strlen("asdf", 1);
+strlen("asdf");

Compact To Variables

Change compact() call to own array

 class SomeClass
 {
     public function run()
     {
         $checkout = 'one';
         $form = 'two';

-        return compact('checkout', 'form');
+        return ['checkout' => $checkout, 'form' => $form];
     }
 }

Unused Foreach Value To Array Keys

Change foreach with unused $value but only $key, to array_keys()

 class SomeClass
 {
     public function run()
     {
         $items = [];
-        foreach ($values as $key => $value) {
+        foreach (array_keys($values) as $key) {
             $items[$key] = null;
         }
     }
 }

Explicit Bool Compare

Make if conditions more explicit

 final class SomeController
 {
     public function run($items)
     {
-        if (!count($items)) {
+        if (count($items) === 0) {
             return 'no items';
         }
     }
 }

Simplify Bool Identical True

Symplify bool value compare to true or false

 class SomeClass
 {
     public function run(bool $value, string $items)
     {
-         $match = in_array($value, $items, TRUE) === TRUE;
-         $match = in_array($value, $items, TRUE) !== FALSE;
+         $match = in_array($value, $items, TRUE);
+         $match = in_array($value, $items, TRUE);
     }
 }

Use Interface Over Implementation In Constructors

Use interface instead of specific class

 class SomeClass
 {
-    public function __construct(SomeImplementation $someImplementation)
+    public function __construct(SomeInterface $someImplementation)
     {
     }
 }

 class SomeImplementation implements SomeInterface
 {
 }

 interface SomeInterface
 {
 }

Shorten Else Ifs

Shortens else/if to elseif

 class SomeClass
 {
     public function run()
     {
         if ($cond1) {
             return $action1;
-        } else {
-            if ($cond2) {
-                return $action2;
-            }
+        } elseif ($cond2) {
+            return $action2;
         }
     }
 }

Unnecessary Ternary Expressions

Remove unnecessary ternary expressions.

-$foo === $bar ? true : false;
+$foo === $bar;

Intval To Type Cast

Change intval() to faster and readable (int) $value

 class SomeClass
 {
     public function run($value)
     {
-        return intval($value);
+        return (int) $value;
     }
 }

In Array And Array Keys To Array Key Exists

Simplify in_array and array_keys functions combination into array_key_exists when array_keys has one argument only

-in_array("key", array_keys($array), true);
+array_key_exists("key", $array);

Simplify DeMorgan Binarys

Simplify negated conditions with de Morgan theorem

 <?php

 $a = 5;
 $b = 10;
-$result = !($a > 20 || $b <= 50);
+$result = $a <= 20 && $b > 50;

@codecov
Copy link

codecov bot commented Jun 2, 2020

Codecov Report

Merging #1371 into master will decrease coverage by 0.02%.
The diff coverage is 20.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1371      +/-   ##
============================================
- Coverage     27.58%   27.56%   -0.03%     
+ Complexity     4248     4246       -2     
============================================
  Files           304      304              
  Lines         13350    13348       -2     
============================================
- Hits           3683     3679       -4     
- Misses         9667     9669       +2     
Impacted Files Coverage Δ Complexity Δ
app/Bots/IRCAnnounceBot.php 0.00% <ø> (ø) 16.00 <0.00> (ø)
app/Console/Commands/AutoNerdStat.php 5.66% <0.00%> (ø) 3.00 <0.00> (ø)
app/Console/Commands/DemoSeed.php 9.75% <0.00%> (ø) 10.00 <0.00> (ø)
app/Helpers/BBCodeConverter.php 0.00% <ø> (ø) 56.00 <0.00> (ø)
app/Helpers/Markdown.php 0.00% <0.00%> (ø) 229.00 <0.00> (ø)
app/Helpers/MediaInfo.php 0.00% <0.00%> (ø) 119.00 <0.00> (ø)
...pp/Http/Controllers/Auth/ApplicationController.php 71.42% <0.00%> (ø) 6.00 <0.00> (ø)
app/Http/Controllers/Auth/RegisterController.php 0.00% <0.00%> (ø) 16.00 <0.00> (ø)
app/Http/Controllers/PlaylistController.php 0.00% <0.00%> (ø) 26.00 <0.00> (ø)
app/Http/Controllers/PollController.php 0.00% <0.00%> (ø) 8.00 <0.00> (ø)
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53fd641...fc51429. Read the comment docs.

@HDVinnie HDVinnie merged commit af4d1e7 into master Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant