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

Remove EqualsAvoidNull check from Qulice #807

Closed
mkordas opened this issue Aug 28, 2016 · 15 comments
Closed

Remove EqualsAvoidNull check from Qulice #807

mkordas opened this issue Aug 28, 2016 · 15 comments
Labels

Comments

@mkordas
Copy link
Contributor

mkordas commented Aug 28, 2016

Currently Qulice prohibits code like

input.equals("contents")

in favour of

"contents".equals(input)

The reason behind this is to avoid null pointer exceptions when input is null. However, IMO it's just hiding a problem, while code should be fragile. Moreover, in well written code input should never be null anyway.

Second reason to remove this check is that inverted notation is unnatural and less readable.

In scope of this issue I expect that check is removed and test is added proving that first syntax is allowed.

@krzyk
Copy link
Collaborator

krzyk commented Aug 28, 2016

@davvd valid bug

@davvd
Copy link

davvd commented Aug 29, 2016

@davvd valid bug

@krzyk added "bug" tag to this issue

@davvd davvd added the bug label Aug 29, 2016
@davvd
Copy link

davvd commented Aug 29, 2016

@mkordas thanks for the report, I topped your acc for 15 mins, payment ID 57c3fe5a6d5f4b008819f535

@davvd davvd added DEV labels Aug 29, 2016
kanghj added a commit to kanghj/qulice that referenced this issue Sep 4, 2016
@kanghj
Copy link
Contributor

kanghj commented Sep 4, 2016

@davvd @mkordas I have made PR #809 for this.

@mkordas
Copy link
Contributor Author

mkordas commented Sep 4, 2016

@kanghj #807 is this issue :)

@kanghj
Copy link
Contributor

kanghj commented Sep 4, 2016

@mkordas, my bad! Edited the original comment

@davvd
Copy link

davvd commented Sep 5, 2016

@davvd @mkordas I have made PR #809 for this.

@kanghj thank you!

@krzyk
Copy link
Collaborator

krzyk commented Apr 27, 2018

@0crat in

@0crat 0crat added the scope label Apr 27, 2018
@0crat
Copy link
Collaborator

0crat commented Apr 27, 2018

@0crat in (here)

@krzyk Job #807 is now in scope, role is DEV

@0crat
Copy link
Collaborator

0crat commented Apr 27, 2018

@0crat in (here)

@krzyk Thanks for your contribution, @mkordas/z! If you would be a member of the project, you would now earn +15 reputation points, as explained in §29. You can join and apply to it, see §2.

@krzyk
Copy link
Collaborator

krzyk commented Apr 27, 2018

@mkordas Change was merged in #809, please close

@0crat
Copy link
Collaborator

0crat commented Apr 27, 2018

@krzyk/z everybody who has role DEV are banned at this job; I won't be able to assign anyone automatically; consider assigning someone manually (as in §19), or invite more people (as in §51), or remove the job from the scope (as in §14)

@0crat
Copy link
Collaborator

0crat commented May 3, 2018

@krzyk/z everybody who has role DEV are banned at this job; I won't be able to assign anyone automatically; consider assigning someone manually (as in §19), or invite more people (as in §51), or remove the job from the scope (as in §14)

@mkordas
Copy link
Contributor Author

mkordas commented May 7, 2018

@krzyk thanks for merging!

@mkordas mkordas closed this as completed May 7, 2018
@0crat 0crat removed the scope label May 7, 2018
@0crat
Copy link
Collaborator

0crat commented May 7, 2018

The job #807 is now out of scope

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

No branches or pull requests

5 participants