Skip to content

Conversation

@geekcom
Copy link

@geekcom geekcom commented Jun 4, 2017

else condition within that method of yours is either unnecessary or redundant.

@morloderex
Copy link
Contributor

@geekcom Tests is failing now..

$this->items[$key] = $value;
}

$this->items[$key] = $value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is always going to be executed now

Copy link

@ghost ghost Jun 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geekcom: you could've only done that shortening if you were breaking the path of execution (using guard clauses for instance). I wouldn't recommend that though, since it alters type signature in this case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let's see what happens with this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you broke the functionality of at least 1 method for no reason at all ... what do you think is going to happen? :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

humm, that's make sense, so sorry

@lagbox
Copy link
Contributor

lagbox commented Jun 4, 2017

In short, there is nothing 'unnecessary or redundant' about this else.

@geekcom geekcom changed the title else unnecessary in this snippet [5.4] else unnecessary in this snippet Jun 4, 2017
usort($items, function () {
return rand(-1, 1);
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again ... this is now always going to be executed

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

Successfully merging this pull request may close these issues.

3 participants