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 File#empty? and File#zero? methods #4295

Closed
wants to merge 1 commit into from
Closed

Add File#empty? and File#zero? methods #4295

wants to merge 1 commit into from

Conversation

maclover7
Copy link

TODO:

  • get tests passing
  • add support for IO objects as argument to method (and add tests for this behavior)

@kares
Copy link
Member

kares commented Nov 15, 2016

seems that its just not compilable:
src/main/java/org/jruby/RubyFile.java:[1532,28] error: no suitable method found for newFileStat(String)

@enebo
Copy link
Member

enebo commented Nov 15, 2016

@maclover7 @kares Amazingly someone else had submitted a PR before you and I landed it but perhaps a single stat is better than an exist? + stat() hmm. exist? on java.io.File is quicker than us performing a stat via jnr-posix but I am guessing more than half the time you expect the file to exist so you are doing two things...

@enebo
Copy link
Member

enebo commented Nov 15, 2016

The other PR probably works better on a minutes reflection as this mechanism will raise an ENOENT or ENODIR I think if the file does not actually exist in stat.setup().

@kcdragon @maclover7 what would work and if either of you are up for it would be to do something like:

file = JRubyFile.createResource(runtime, filename);
if file == null then 
  false
else 
  file.stat.exist?
end

We try really hard to avoid exception when we can as they are expensive. This above approach will eliminate the original exist() + stat() calling on existing file (which is two stat() calls) to a single one.

@enebo enebo added this to the Invalid or Duplicate milestone Nov 15, 2016
@enebo
Copy link
Member

enebo commented Nov 15, 2016

Open a new PR if either of you want to take that on. I am closing this one.

@enebo enebo closed this Nov 15, 2016
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.

None yet

3 participants