From 33a9ef064ef3697100199332e53d30fb41fe03e1 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 7 Jul 2015 10:15:15 +0200 Subject: [PATCH 1/7] Allow only objects implementing ::dispose --- spec/composite-disposable-spec.coffee | 7 +++++++ src/composite-disposable.coffee | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/spec/composite-disposable-spec.coffee b/spec/composite-disposable-spec.coffee index 880e13e..dac921a 100644 --- a/spec/composite-disposable-spec.coffee +++ b/spec/composite-disposable-spec.coffee @@ -29,3 +29,10 @@ describe "CompositeDisposable", -> expect(disposable1.disposed).toBe true expect(disposable2.disposed).toBe false expect(disposable3.disposed).toBe true + + it "denies to add an object that does not respond to ::dispose", -> + composite = new CompositeDisposable + + expect(-> composite.add(undefined)).toThrow() + expect(-> composite.add(null)).toThrow() + expect(-> composite.add(whatever: ->)).toThrow() diff --git a/src/composite-disposable.coffee b/src/composite-disposable.coffee index 6ae38be..7f60406 100644 --- a/src/composite-disposable.coffee +++ b/src/composite-disposable.coffee @@ -54,7 +54,11 @@ class CompositeDisposable # method. add: -> unless @disposed - @disposables.add(disposable) for disposable in arguments + for disposable in arguments + if typeof disposable?.dispose isnt "function" + throw new Error("#{disposable} must implement ::dispose!") + + @disposables.add(disposable) return # Public: Remove a previously added disposable. From 900fc55b7c67865bb3498b190eccaf357dff6061 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 7 Jul 2015 14:10:28 +0200 Subject: [PATCH 2/7] :art: --- spec/composite-disposable-spec.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/composite-disposable-spec.coffee b/spec/composite-disposable-spec.coffee index dac921a..126686c 100644 --- a/spec/composite-disposable-spec.coffee +++ b/spec/composite-disposable-spec.coffee @@ -30,7 +30,7 @@ describe "CompositeDisposable", -> expect(disposable2.disposed).toBe false expect(disposable3.disposed).toBe true - it "denies to add an object that does not respond to ::dispose", -> + it "throws an error if a disposable does not implement ::dispose", -> composite = new CompositeDisposable expect(-> composite.add(undefined)).toThrow() From c19348df4b69867a000c40bc922ad6e98d6f6a7d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 7 Jul 2015 19:55:02 +0200 Subject: [PATCH 3/7] Ensure disposable on `dispose` --- spec/composite-disposable-spec.coffee | 17 +++++++++++++---- src/composite-disposable.coffee | 13 +++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/spec/composite-disposable-spec.coffee b/spec/composite-disposable-spec.coffee index 126686c..6bfbc3d 100644 --- a/spec/composite-disposable-spec.coffee +++ b/spec/composite-disposable-spec.coffee @@ -30,9 +30,18 @@ describe "CompositeDisposable", -> expect(disposable2.disposed).toBe false expect(disposable3.disposed).toBe true - it "throws an error if a disposable does not implement ::dispose", -> + it "throws an error when adding a disposable without a ::dispose function", -> composite = new CompositeDisposable - expect(-> composite.add(undefined)).toThrow() - expect(-> composite.add(null)).toThrow() - expect(-> composite.add(whatever: ->)).toThrow() + expect(-> composite.add(undefined)).toThrow("undefined must implement ::dispose!") + expect(-> composite.add(null)).toThrow("null must implement ::dispose!") + expect(-> composite.add(whatever: ->)).toThrow("[object Object] must implement ::dispose!") + + it "throws an error when disposing a disposable without a ::dispose function", -> + composite = new CompositeDisposable + disposable = {dispose: ->} + composite.add(disposable) + + delete disposable["dispose"] + + expect(-> composite.dispose()).toThrow("[object Object] must implement ::dispose!") diff --git a/src/composite-disposable.coffee b/src/composite-disposable.coffee index 7f60406..6956646 100644 --- a/src/composite-disposable.coffee +++ b/src/composite-disposable.coffee @@ -37,7 +37,8 @@ class CompositeDisposable dispose: -> unless @disposed @disposed = true - @disposables.forEach (disposable) -> + @disposables.forEach (disposable) => + @ensureDisposable(disposable) disposable.dispose() @disposables = null return @@ -55,9 +56,7 @@ class CompositeDisposable add: -> unless @disposed for disposable in arguments - if typeof disposable?.dispose isnt "function" - throw new Error("#{disposable} must implement ::dispose!") - + @ensureDisposable(disposable) @disposables.add(disposable) return @@ -74,3 +73,9 @@ class CompositeDisposable clear: -> @disposables.clear() unless @disposed return + + # Private: Ensures that the given `disposable` responds to ::dispose and + # throws an error if it doesn't. + ensureDisposable: (disposable) -> + if typeof disposable?.dispose isnt "function" + throw new Error("#{disposable} must implement ::dispose!") From 75bd4b8fb560e23b645306639853b9ea170a183f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 8 Jul 2015 12:53:18 +0200 Subject: [PATCH 4/7] Revert disposable guard and introduce `addIfDisposable` --- spec/composite-disposable-spec.coffee | 20 +++++++------------- src/composite-disposable.coffee | 15 ++++++--------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/spec/composite-disposable-spec.coffee b/spec/composite-disposable-spec.coffee index 6bfbc3d..0deedf8 100644 --- a/spec/composite-disposable-spec.coffee +++ b/spec/composite-disposable-spec.coffee @@ -30,18 +30,12 @@ describe "CompositeDisposable", -> expect(disposable2.disposed).toBe false expect(disposable3.disposed).toBe true - it "throws an error when adding a disposable without a ::dispose function", -> - composite = new CompositeDisposable - - expect(-> composite.add(undefined)).toThrow("undefined must implement ::dispose!") - expect(-> composite.add(null)).toThrow("null must implement ::dispose!") - expect(-> composite.add(whatever: ->)).toThrow("[object Object] must implement ::dispose!") - - it "throws an error when disposing a disposable without a ::dispose function", -> - composite = new CompositeDisposable - disposable = {dispose: ->} - composite.add(disposable) + describe "::addIfDisposable(args...)", -> + it "ignores disposables without a ::dispose function", -> + composite = new CompositeDisposable + composite.addIfDisposable(disposable1, undefined, null) + composite.addIfDisposable(whatever: ->) - delete disposable["dispose"] + composite.dispose() - expect(-> composite.dispose()).toThrow("[object Object] must implement ::dispose!") + expect(disposable1.disposed).toBe true diff --git a/src/composite-disposable.coffee b/src/composite-disposable.coffee index 6956646..393f487 100644 --- a/src/composite-disposable.coffee +++ b/src/composite-disposable.coffee @@ -38,7 +38,6 @@ class CompositeDisposable unless @disposed @disposed = true @disposables.forEach (disposable) => - @ensureDisposable(disposable) disposable.dispose() @disposables = null return @@ -55,8 +54,12 @@ class CompositeDisposable # method. add: -> unless @disposed - for disposable in arguments - @ensureDisposable(disposable) + @disposables.add(disposable) for disposable in arguments + return + + addIfDisposable: -> + unless @disposed + for disposable in arguments when typeof disposable?.dispose is "function" @disposables.add(disposable) return @@ -73,9 +76,3 @@ class CompositeDisposable clear: -> @disposables.clear() unless @disposed return - - # Private: Ensures that the given `disposable` responds to ::dispose and - # throws an error if it doesn't. - ensureDisposable: (disposable) -> - if typeof disposable?.dispose isnt "function" - throw new Error("#{disposable} must implement ::dispose!") From 89328531cd7c028aa87421bdfadb277a67bcbc2d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 8 Jul 2015 12:53:58 +0200 Subject: [PATCH 5/7] :art: --- src/composite-disposable.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/composite-disposable.coffee b/src/composite-disposable.coffee index 393f487..ed5b7be 100644 --- a/src/composite-disposable.coffee +++ b/src/composite-disposable.coffee @@ -37,7 +37,7 @@ class CompositeDisposable dispose: -> unless @disposed @disposed = true - @disposables.forEach (disposable) => + @disposables.forEach (disposable) -> disposable.dispose() @disposables = null return From 2396bc091072e96e9055d7f23a47b4d0f0c37b3b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 8 Jul 2015 13:02:40 +0200 Subject: [PATCH 6/7] :memo: Write docs --- src/composite-disposable.coffee | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/composite-disposable.coffee b/src/composite-disposable.coffee index ed5b7be..9e906d1 100644 --- a/src/composite-disposable.coffee +++ b/src/composite-disposable.coffee @@ -57,10 +57,16 @@ class CompositeDisposable @disposables.add(disposable) for disposable in arguments return + # Public: Add a disposable to be disposed when the composite is disposed, + # ignoring those ones that do not implement `.dispose()`. + # + # See {CompositeDisposable::add} documentation for further information. + # + # * `disposable` {Disposable} instance or any object with a `.dispose()` + # method. addIfDisposable: -> - unless @disposed - for disposable in arguments when typeof disposable?.dispose is "function" - @disposables.add(disposable) + for disposable in arguments + @add(disposable) if typeof disposable?.dispose is "function" return # Public: Remove a previously added disposable. From 765bb14290989c66bec59cac81aff480bac88053 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 8 Jul 2015 13:04:51 +0200 Subject: [PATCH 7/7] :memo: --- spec/composite-disposable-spec.coffee | 2 +- src/composite-disposable.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/composite-disposable-spec.coffee b/spec/composite-disposable-spec.coffee index 0deedf8..0b9799b 100644 --- a/spec/composite-disposable-spec.coffee +++ b/spec/composite-disposable-spec.coffee @@ -30,7 +30,7 @@ describe "CompositeDisposable", -> expect(disposable2.disposed).toBe false expect(disposable3.disposed).toBe true - describe "::addIfDisposable(args...)", -> + describe "::addIfDisposable(disposables...)", -> it "ignores disposables without a ::dispose function", -> composite = new CompositeDisposable composite.addIfDisposable(disposable1, undefined, null) diff --git a/src/composite-disposable.coffee b/src/composite-disposable.coffee index 9e906d1..eb36719 100644 --- a/src/composite-disposable.coffee +++ b/src/composite-disposable.coffee @@ -58,7 +58,7 @@ class CompositeDisposable return # Public: Add a disposable to be disposed when the composite is disposed, - # ignoring those ones that do not implement `.dispose()`. + # ignoring it if it does not implement `.dispose()`. # # See {CompositeDisposable::add} documentation for further information. #