Crystal: Remove class Tempfile

[straight-shoota]

Tempfile is actually identical to File except for the constructor (which can be put in File with a different name), a few class methods (where this is also true), and the instance methods delete and unlink which are just aliases to File.delete(@path).

I don't see a point in having a special type for this as the behaviour is identical to File, it's just an alternate way of initializing the instance.

The class methods/constructors could be renamed as followed:

• Tempfile.new -> File.tempfile
• Tempfile.open -> File.tempfile (yield)
• Tempfile.dirname -> Dir.tempdir
• Tempfile.tempname -> File.tempname

This issue is loosely related to #5938 about adjusting the arguments to the class methods/constructors but that's independent of the actual names.

[RX14]

Yes please! We can also discuss the merits of just putting delete on File.

[sdogruyol]

Yeah! Wish I could upvote this more than once :+1:

[straight-shoota]

On a second thought: Dir.tempdir is not a that a good name because you would assume it behaves the same as File.tempfile for folders (that feature is actually proposed in #4096). But I have no better idea of how to name these... Any thoughts?

[ysbaddaden]

Counter arguments:

1. it introduces a redundant naming —I mean FILE.tempFILE... seriously?;
2. it introduces yet-another construct to learn, instead of relying on the well known .open paradigm —furthermore File.tempfile { |file| } ...;
3. it pushes yet-another concept into File —I thought it was bloated enough?

What would be so wrong with Tempfile simply inheriting from File?

[straight-shoota]

Classes define behaviour. Having a subclass with identical behaviour to the superclass makes no sense. I'd rather have a few more methods in File than another useless subclass.

Whether you have to learn Tempfile.new/Tempfile.open Vs. File.tempfile makes no difference to me. Both concepts, new + open and yielding overloads, are common API patterns in the stdlib.

[ysbaddaden]

I believe behaviors are different:

• File needs a filename to open/create;
• Tempfile provides a random filename and eventually does some cleanup.

I.e. Tempfile is a specialization of File. It can override a few methods and transparently be used in place of a File, yet having a Tempfile makes it explicit that you use on a temporary file, it's not any File.

[straight-shoota]

But what's the difference between a file and a temporary file besides the initialization? And what cleanup do you mean?

[Sija]

And what cleanup do you mean?

I guess @ysbaddaden referred to Tempfile#delete/unlink.

[straight-shoota]

But that's not specific to a tempfile. If that method it to exist, it should be on File.

[Sija]

@straight-shoota Perhaps it should, but ATM it is specific to Tempfile.

[straight-shoota]

Yes, but there is no conceptual difference between deleting a file and deleting a tempfile. So it's not a valid argument for Tempfile having different behaviour.

It's just a question of API design and this discussion is about removing that class - and maybe adding File#delete in return.

[RX14]

If it's only the way that the file is constructed that differs between a temporary file and a normal file, then that should be handled by differently-named constructors.

.open is a pattern of having a method which yields a resource to the block then cleans up in an ensure. Nothing to do with naming. Or should we enforce all constructors are named .new?

[asterite]

I agree, Tempfile is not needed.

[asterite]

Moved to status:accepted. If anyone is going to work on this, please let us know (or mark yourself as asignee if you are on the core team).

[yxhuvud]

If you want to keep the pattern with open, then there are alternatives that let you keep that, from #open_tempfile to File.open(temp: true) or any number of variants along those lines.

[straight-shoota]

I'll be happy to work on this.

[RX14]

i wish github would allow you to assign people outside the core team...

[asterite]

@RX14 I just did that ;-)

It appears in the dropbox for some reason for me... it doesn't for you?

[RX14]

oh, i think it's just because @straight-shoota is the author of the issue... You can't assign arbitrary issues to arbitrary people

[straight-shoota]

In order to implement this:

• I'd like to get some feedback on #5938
• #5951 should be merged