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

Add #empty? to Tempfile, StringIO, File::Stat #1759

Open
wants to merge 1 commit into
base: trunk
from

Conversation

4 participants
@mikegee
Copy link

commented Nov 16, 2017

Rubocop prefers empty? over length == 0 and size == 0, which is great for String, Array, Hash, etc. It would be nice if more classes implemented #empty? for consistency.

See related discussion at rubocop-hq/rubocop#2841

https://bugs.ruby-lang.org/issues/14136

@mikegee mikegee force-pushed the mikegee:add-some-empty-checks branch from fc1f398 to c717414 Nov 16, 2017

@mikegee

This comment has been minimized.

Copy link
Author

commented Nov 16, 2017

I couldn't find where to add tests for File::Stat#empty?, but that one just an alias. Maybe it isn't valuable enough to test it. ¯\_(ツ)_/¯

rb_raise(rb_eIOError, "not opened");
}
long len = RSTRING_LEN(string);
if (len == 0) { return Qtrue; }

This comment has been minimized.

Copy link
@esparta

esparta Nov 17, 2017

Contributor

Since len is not used anywhere else but that if, can this be equivalent?

if (RSTRING_LEN(string) == 0) { return Qtrue; }

This comment has been minimized.

Copy link
@mikegee

mikegee Nov 17, 2017

Author

Good point!

Add #empty? to Tempfile, StringIO, File::Stat
Rubocop prefers `empty?` over `length == 0` and `size == 0`, which is great for String, Array, Hash, etc. It would be nice if more classes implemented `#empty?` for consistancy.

See related discussion at rubocop-hq/rubocop#2841

@mikegee mikegee force-pushed the mikegee:add-some-empty-checks branch from c717414 to 7a939cc Nov 17, 2017

@matzbot matzbot force-pushed the ruby:trunk branch from 2677ddd to ce7ad3a Jan 18, 2018

@Drenmi

This comment has been minimized.

Copy link

commented Sep 12, 2018

Would love to see this merged in the name of 🦆 typing. 🙂

@bbatsov

This comment has been minimized.

Copy link

commented Sep 12, 2018

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.