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.writeAsLines/writeAsLinesSync methods #42424

Open
jamesderlin opened this issue Jun 20, 2020 · 5 comments
Open

Add File.writeAsLines/writeAsLinesSync methods #42424

jamesderlin opened this issue Jun 20, 2020 · 5 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-enhancement A request for a change that isn't a bug

Comments

@jamesderlin
Copy link
Contributor

File has readAsBytes and writeAsBytes, readAsString and writeAsString, and readAsLines but no corresponding writeAsLines.

It might seem like it'd be trivial to instead use writeAsString(lines.join('\n')). However:

  1. The output from writeAsString(lines.join('\n')) might not technically qualify as a "text file" according to the POSIX definition. POSIX defines a "text file" to be a collection of lines, and a line is defined as having a newline terminator. (Also see https://unix.stackexchange.com/questions/446237/ and https://stackoverflow.com/questions/729692/.)

  2. If writeAsLines would be considered trivial, then the same argument could have applied to readAsLines. (For similar reasons regarding a final newline, readAsLines is not quite the same as readAsString().split('\n').) Arguably it's more important for reading to be tolerant and for writing to be strict, which implies that there should be a standard way of writing text files.

(I know I'm being pedantic; I'm not particularly optimistic about this being addressed.)

@lrhn
Copy link
Member

lrhn commented Jun 20, 2020

The writeAsLines functionality can be achieved by doing:

var out = file.openWrite();
for (var line in lines) out.writeln(line);
out.close();

so the need for a single writeAsLines(List<String> lines) method isn't as great.

For readAsLines, there is no corresponding readln method on the input. You would have to read the input and manually split it, and for asynchronous reading, you would have to combine chunks correctly and then split on some choice of line terminators. It's just more complex to read than to write.

Admittedly, writeAsString is also just file.openWrite()..write(string)..close(); and writeAsBytes is file.openWrite()..add(bytes)..close();, so that's not much of an argument.
It's more likely that the need for the use-case just wasn't obvious to the original designers.

Adding any method to any interface is currently a breaking change, so adding the method will probably only happen if we consider having it worth the cost. With the rather easy workaround above, I wouldn't be too optimistic either.

If we ever rework the dart:io API, it's worth remembering this.

@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-enhancement A request for a change that isn't a bug labels Jun 20, 2020
@sigurdm
Copy link
Contributor

sigurdm commented Jun 22, 2020

Adding any method to any interface is currently a breaking change,

But it could be done as an extension?

@lrhn
Copy link
Member

lrhn commented Jun 22, 2020

Sure. I'm not particularly fond of simply adding extensions methods for anything that we might want to add to an API that we control ourselves.
It means that some methods are instance methods and other methods are extension methods, and the only real reason for the difference is "added before/after 2.0". That's not good design.

(Now, if we had interface default methods, I'd be much happier adding some of those, and changing existing methods to be such, because that would be a backwards and forwards compatible change where all the methods are still instance methods).

@sigurdm
Copy link
Contributor

sigurdm commented Jun 22, 2020

That's not good design.

I follow the sentiment.
Question is how important 'clean' design is vs. how useful our standard libraries are.

We did not have extensions before 2.x. One could argue that writeAsString should also be an extension as it should (always?) be implemented in terms of other primitives from File. As such it is a disadvantage that writeAsString is an instance method - but that is too late to change.

Are there technical disadvantages to writeAsLines being an extension method, besides revealing our lack of foresight?

Now, if we had interface default methods...

I had not heard of these before. Is there a proposal for this already?

@lrhn
Copy link
Member

lrhn commented Jun 22, 2020

Not a proposal as such, but there is a request.
(And I have a proposal written, just haven't uploaded it yet).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants