I know this will sound mildly icky but I prefer that we pass ThreadContext instead of Ruby in most cases. When you need to call a ruby method you must get ThreadContext and Ruby.getCurrentContext() is an expensive method. If you go the opposite direction getting Ruby from ThreadContext is only a field context.runtime. In your case (atm), you don't need to call a Ruby method from a *Resource; but I think ThreadContext will be a bit more futureproof.
With that said I suspect originally we tried to decouple this code from having Runtime knowledge but I see no important reason why we should have this decoupling. So it seems fine to me. -Tom On Wed, Feb 19, 2014 at 4:53 PM, Dmitry Ratnikov <ratni...@gmail.com> wrote: > Hey all, > > This in regards of replacing NonExistentFile.NOT_EXIST check with a > special EmptyFileResource (or NotExistsFileResource, if you strongly > prefer): > https://github.com/jruby/jruby/blob/jruby-1_7/core/src/main/java/org/jruby/util/JRubyFile.java#L67 > > What I realized is that some of the filestat methods (e.g. size, > lastModified and others) should raise ErrorNoENOENTError like MRI does, but > I don't have a reference to Ruby object to raise it from. Technically, I > can probably get away with throwing some sort of RTE (e.g. > OperationUnsupportedException) since I'm pretty sure FileTest checks for > existence and raises "Not found" error for me, but that sounds ugly. > > Before I make JRubyFile.createResource(Ruby, String) method mandatory, > kill createResource(String, String) and will have to rewrite most of > org.jruby.util.Dir to conform to the new interface, maybe you have some > other suggestions? > > Thanks! > -- blog: http://blog.enebo.com twitter: tom_enebo mail: tom.en...@gmail.com
