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

dart:io should use Futures not "handlers" #1786

Closed
DartBot opened this issue Feb 21, 2012 · 6 comments
Closed

dart:io should use Futures not "handlers" #1786

DartBot opened this issue Feb 21, 2012 · 6 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Feb 21, 2012

This issue was originally filed by @seaneagan


dart:io currently uses "handlers" for async operations instead of the standard Future API. This is inconsistent and less powerful. For example, File could look like:

interface File {
  //...
  Future<void> create();
  Future<void> delete();
  Future<bool> exists();
  Future<String> fullPath();
  Future<RandomAccessFile> open([FileMode mode]);
}

//...

file.exists().then((bool exists) => print(exists ? "yes" : "no"));

// and replace createSync, deleteSync, etc. with issue #104
print(await file.exists() ? "yes" : "no");

@iposva-google
Copy link
Contributor

cc @madsager.
cc @sgjesse.
Added Area-IO, Triaged labels.

@madsager
Copy link
Contributor

I don't think the handlers are any less powerful. In fact I think the handlers is a lower-level API on top of which you can build the Future API if you so desire. See the example below.

Futures is not a good match for the streaming APIs for example. At this point we are keeping the APIs low-level but consistent. The APIs might change but moving to Futures is not a priority at this point.

#import('dart:io');

class FutureFile {
  File _file;
  
  FutureFile(String path) {
    _file = new File(path);
  }
  
  Future<bool> exists() {
    Completer completer = new Completer();
    _file.exists();
    _file.existsHandler = (result) {
      completer.complete(result);
    };
    // TODO: add error handler that completes with an error.
    return completer.future;
  }
}

void main() {
  FutureFile f = new FutureFile('flaf.txt');
  f.exists().then((exists) => print(exists));
}


Removed Type-Defect, Priority-Medium labels.
Added Type-Enhancement, Priority-Low labels.

@DartBot
Copy link
Author

DartBot commented Feb 27, 2012

This comment was originally written by @seaneagan


Async method completion (e.g. File#{create,delete,exists,etc.}) should be localized to each call, not set on the File (or similar) objects. Otherwise when you call an async method, you need to check that any previous call has been completed before changing the associated "handler" property to whatever you need for the new call. Thus it cannot be used for concurrent file operations of the same type. One would be forced to use separate File objects for each operation, thus losing any low-level gains from not using Futures, not to mention more verbose and confusing code. The situation is even worse with error handlers being shared across multiple operation types.

Thus, if Futures are too much overhead, the async methods should accept success and (optional) error callback functions. For events which occur multiple times not in response to a particular asyn method call (e.g. lineHandler, dataHandler, errorHandler), however, are correctly placed on the Stream objects, and if full-on Event objects are too much overhead then "handler" properties work just fine there.

@DartBot
Copy link
Author

DartBot commented Feb 27, 2012

This comment was originally written by amatiasq...@gmail.com


Agree with Comment#­3 @­seaneagan1

Good summary.

@madsager
Copy link
Contributor

Added Fixed label.

@kevmoo
Copy link
Member

kevmoo commented May 14, 2014

Removed Area-IO label.
Added Area-Library, Library-IO labels.

@DartBot DartBot added Type-Enhancement P3 A lower priority bug or feature request area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels May 14, 2014
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed type-enhancement labels Mar 1, 2016
copybara-service bot pushed a commit that referenced this issue Dec 5, 2022
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/dc502d0..4d7dc93):
  4d7dc93b  2022-12-05  dependabot[bot]  Bump github/codeql-action from 2.1.31 to 2.1.35 (#3263)
  bcf8b6e8  2022-12-05  Parker Lougheed  Weight enums the same as classes for searching (#3260)
  7d95578b  2022-12-04  Parker Lougheed  Update template descriptions (#3258)
  d558f043  2022-12-04  Parker Lougheed  Fix error when using base element href (#3256)
  c3663762  2022-12-04  Parker Lougheed  Add unnecessary override ignore to fix build (#3257)

http (https://github.com/dart-lang/http/compare/976bd56..46a7708):
  46a7708  2022-12-02  Brian Quinlan  Remove binary artifact (#833)

sync_http (https://github.com/dart-lang/sync_http/compare/f5c1f18..8622614):
  8622614  2022-12-02  Kevin Moore  blast_repo fixes (#32)

test (https://github.com/dart-lang/test/compare/f3d80a6..4dceb87):
  4dceb87c  2022-12-01  Nate Bosch  Ignore some usage of dperecated errors (#1807)

webdev (https://github.com/dart-lang/webdev/compare/91b8a19..e39506e):
  e39506e  2022-12-05  Anna Gringauze  Pre-warm expression compiler to speed up Flutter Inspector page loading. (#1786)
  9b19b3b  2022-12-02  Elliott Brooks (she/her)  Can save storage objects in both `session` and `local` storage (#1807)
  e75c45e  2022-12-02  Elliott Brooks (she/her)  Injected client adds `isFlutterApp` to global window object (#1806)
  ba5e3ec  2022-12-01  Elliott Brooks (she/her)  `DebugSession` listens to events instead of just sending events (#1804)

Change-Id: I881d02e966b763879df72b29653a9f241b71eb3d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/273826
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
This issue was closed.
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 P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants