Enhance conditions #3

peter279k · 2018-12-01T07:38:19Z

Changed log

To reduce nested condition, using the ?: operator to do that.
Using the ?? syntax to reduce conditions.
Using the error Exception first to enhance condition logic.

coveralls · 2018-12-01T07:40:02Z

Coverage decreased (-0.1%) to 94.427% when pulling 7fee5ac on peter279k:enhance_condition into b8acd4b on brick:master.

BenMorel · 2018-12-01T10:04:11Z

src/Iterator/CsvJsonFileIterator.php

+        $this->handle = (is_resource($file) ? $file : @ fopen($file, 'rb'));
+
+        if ($this->handle === false) {
+            throw new \InvalidArgumentException('Cannot open file for reading: ' . $file);


What do you mean about this comment?

I don't like the null coalesce operator here, as it also allows for undefined variable without notice. This operator is equivalent to isset($values) ? $values : [], not strictly equivalent to the original code.

This condition is not related to the null coalesce operator.

I think this change is proper approach.

BenMorel · 2018-12-01T10:06:20Z

src/ObjectArrayStorage.php

@@ -49,7 +49,7 @@ public function get($object) : array
    {
        $values = $this->storage->get($object);

-        return ($values === null) ? [] : $values;
+        return $values ?? [];


I don't like the null coalesce operator here, as it also allows for undefined variable without notice. This operator is equivalent to isset($values) ? $values : [], not strictly equivalent to the original code.

BenMorel · 2018-12-01T10:07:16Z

src/ObjectStorage.php

        }

-        throw new \UnexpectedValueException('Object not found.');
+        return $this->data[$hash];


This change brings no value.

This approach is about error exception first.

It's about change the order of conditions.

I got that, but what's the point? Please don't spent time on nitpick like this :)

Ok. I will not spend time about this change.

BenMorel · 2018-12-01T10:09:30Z

src/ObjectStorage.php

-        }
-
-        return null;
+        return $this->data[$hash] ?? null;


You can keep this change alone.

How about reverting this due to your previous comment.

This is an isset($x) ? $x : null, so conceptually equivalent to $x ?? null. I can accept this change.

Enhance conditions

41b6863

BenMorel reviewed Dec 1, 2018

View reviewed changes

Change into original condition approach

7fee5ac

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance conditions #3

Enhance conditions #3

peter279k commented Dec 1, 2018

coveralls commented Dec 1, 2018 •

edited

BenMorel Dec 1, 2018

peter279k Dec 1, 2018

BenMorel Dec 1, 2018

peter279k Dec 2, 2018

BenMorel Dec 1, 2018

BenMorel Dec 1, 2018

peter279k Dec 1, 2018

BenMorel Dec 1, 2018

peter279k Dec 1, 2018 •

edited

BenMorel Dec 1, 2018

peter279k Dec 1, 2018

BenMorel Dec 1, 2018

Enhance conditions #3

Are you sure you want to change the base?

Enhance conditions #3

Conversation

peter279k commented Dec 1, 2018

Changed log

coveralls commented Dec 1, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peter279k Dec 1, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 1, 2018 •

edited

peter279k Dec 1, 2018 •

edited