Struct std.mmfile.MmFile?

[dlangBugzillaToGithub]

bearophile_hugs reported this on 2013-02-12T17:45:01Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=9501

CC List

• issues.dlang (@jmdavis)
• qs.il.paperinik

Description

Maybe it's a good idea for std.mmfile.MmFile to become a struct like std.stdio.File, so the file is closed when this struct goes out of scope and the reference count it keeps is zero.

[dlangBugzillaToGithub]

issues.dlang (@jmdavis) commented on 2013-02-12T17:54:43Z

Yes, std.mmfile.MmFile should be a struct. It doesn't use polymorphism and has no need to be a class. But making it a struct would break all code that uses it, so we're arguably stuck with it being the way that it is.

[dlangBugzillaToGithub]

bearophile_hugs commented on 2013-02-12T18:19:08Z

(In reply to comment #1)
> Yes, std.mmfile.MmFile should be a struct. It doesn't use polymorphism and has
> no need to be a class.

And maybe it doesn't have an explicit close() method, I don't know why, maybe for safety in accessing the memory mapped bytes.


> But making it a struct would break all code that uses
> it, so we're arguably stuck with it being the way that it is.

There are always acceptable deprecation paths, if we want them. Like adding the struct to std.mmfile with another name ("RcMmFile"?) and deprecating std.mmfile.MmFile for two years.

[dlangBugzillaToGithub]

bearophile_hugs commented on 2013-02-12T20:05:43Z

Mostly unrelated, about std.mmfile: I think in the std.mmfile module this line (line 270) should offer a string message too when the file is not found:

errnoEnforce(false);


And I think switches like this one should become "final":

            switch (mode)
            {
            case Mode.read:
...
            default:
                assert(0);
            }

[vandys]

OTOH, if FILE was a class (or interface), MmFile could plug in anywhere a FILE is used. And then a debug/tracing version of a FILE (implemented in a subclass) would also be available, as would one which permits a read-only "FILE" based on string contents. Thsi last would make some unit tests much tidier, leaning only on polymorphism.