Changed my mind about Async.new vs Async.create

Author: jdantonio

lib/concurrent/async.rb

@@ -5,6 +5,7 @@
 require 'concurrent/ivar'
 require 'concurrent/executor/immediate_executor'
 require 'concurrent/executor/serialized_execution'
+require 'concurrent/concern/deprecation'
 
 module Concurrent
 
@@ -30,23 +31,12 @@ module Concurrent
   # object. The former method allows methods to be called asynchronously by posting
   # to the global thread pool. The latter allows a method to be called synchronously
   # on the current thread but does so safely with respect to any pending asynchronous
-  # method calls. Both methods return an `Obligation` which can be inspected for
+  # method calls. Both methods return an `IVar` which can be inspected for
   # the result of the method call. Calling a method with `async` will return a
-  # `:pending` `Obligation` whereas `await` will return a `:complete` `Obligation`.
+  # `:pending` `IVar` whereas `await` will return a `:complete` `IVar`.
   # 
   # Very loosely based on the `async` and `await` keywords in C#.
   # 
-  # ### An Important Note About Initialization
-  # 
-  # > This module depends on several internal stnchronization mechanisms that
-  # > must be initialized prior to calling any of the async/await/executor methods.
-  # > To ensure thread-safe initialization the class `new` method will be made
-  # > private when the `Concurrent::Async` module is included. A factory method
-  # > called `create` will be defined in its place. The `create`factory will
-  # > create a new object instance, passing all arguments to the constructor,
-  # > and will initialize all stnchronization mechanisms. This is the only way
-  # > thread-safe initialization can be guaranteed. 
-  # 
   # ### An Important Note About Thread Safe Guarantees
   # 
   # > Thread safe guarantees can only be made when asynchronous method calls
@@ -70,23 +60,19 @@ module Concurrent
   #       nil
   #     end
   #   end
-  #
-  #   horn = Echo.new #=> NoMethodError: private method `new' called for Echo:Class
   #   
-  #   horn = Echo.create
+  #   horn = Echo.new
   #   horn.echo('zero')      # synchronous, not thread-safe
   #   
   #   horn.async.echo('one') # asynchronous, non-blocking, thread-safe
   #   horn.await.echo('two') # synchronous, blocking, thread-safe
   #
-  # @see Concurrent::Concern::Obligation
   # @see Concurrent::IVar
   module Async
 
-    # @!method self.create(*args, &block)
+    # @!method self.new(*args, &block)
     #
-    #   The factory method used to create new instances of the asynchronous
-    #   class. Used instead of `new` to ensure proper initialization of the
+    #   Instanciate a new object and ensure proper initialization of the
     #   synchronization mechanisms.
     #
     #   @param [Array<Object>] args Zero or more arguments to be passed to the
@@ -129,21 +115,13 @@ def self.validate_argc(obj, method, *args)
     # @!visibility private
     def self.included(base)
       base.singleton_class.send(:alias_method, :original_new, :new)
-      base.send(:private_class_method, :original_new)
       base.extend(ClassMethods)
       super(base)
     end
 
     # @!visibility private
     module ClassMethods
-
-      # @deprecated
       def new(*args, &block)
-        warn '[DEPRECATED] use the `create` method instead'
-        create(*args, &block)
-      end
-
-      def create(*args, &block)
         obj = original_new(*args, &block)
         obj.send(:init_synchronization)
         obj
@@ -280,7 +258,7 @@ def executor=(executor)
     # @!visibility private
     # @deprecated
     def init_mutex
-      warn '[DEPRECATED] use the `create` method instead'
+      deprecated 'mutex synchronization now happens automatically'
       init_synchronization
     rescue InitializationError
       # suppress

spec/concurrent/async_spec.rb

@@ -29,38 +29,20 @@ def with_block
     end
 
     subject do
-      obj = async_class.create
+      obj = async_class.new
       obj.executor = executor
       obj
     end
 
     context 'object creation' do
 
-      # Will be added in 1.0 once deprecated methods are removed
-      #it 'makes #new private' do
-      #  expect{ async_class.new }.to raise_error(NoMethodError)
-      #end
-
-      # Will be removed in 1.0 once deprecated methods are removed
-      it '#new delegates to #create' do
+      it 'delegates to the original constructor' do
         args = [:foo, 'bar', 42]
-        expect(async_class).to receive(:create).once.with(*args)
+        expect(async_class).to receive(:original_new).once.with(*args).and_call_original
         async_class.new(*args)
       end
 
-      it 'uses #create to instanciate new objects' do
-        object = async_class.create
-        expect(object).to be_a async_class
-      end
-
-      specify '#create initializes synchronization' do
-        mock = async_class.create
-        allow(async_class).to receive(:original_new).and_return(mock)
-        expect(mock).to receive(:init_synchronization).once.with(no_args)
-        async_class.create
-      end
-
-      specify '#create passes all args to the constructor' do
+      specify 'passes all args to the original constructor' do
         clazz = Class.new do
           include Concurrent::Async
           attr_reader :args
@@ -69,11 +51,11 @@ def initialize(*args)
           end
         end
 
-        object = clazz.create(:foo, :bar)
+        object = clazz.new(:foo, :bar)
         expect(object.args).to eq [:foo, :bar]
       end
 
-      specify '#create passes all args to the constructor' do
+      specify 'passes a given block to the original constructor' do
         clazz = Class.new do
           include Concurrent::Async
           attr_reader :block
@@ -82,9 +64,16 @@ def initialize(&block)
           end
         end
 
-        object = clazz.create{ 42 }
+        object = clazz.new{ 42 }
         expect(object.block).to eq 42
       end
+
+      specify 'initializes synchronization' do
+        mock = async_class.new
+        allow(async_class).to receive(:original_new).and_return(mock)
+        expect(mock).to receive(:init_synchronization).once.with(no_args)
+        async_class.new
+      end
     end
 
     context '#validate_argc' do
@@ -158,21 +147,21 @@ def many(*args, &block) nil; end
       it 'returns the default executor when #executor= has never been called' do
         expect(Concurrent).to receive(:global_io_executor).
           and_return(ImmediateExecutor.new)
-        subject = async_class.create
+        subject = async_class.new
         subject.async.echo(:foo)
       end
 
       it 'returns the memo after #executor= has been called' do
         executor = ImmediateExecutor.new
         expect(executor).to receive(:post)
-        subject = async_class.create
+        subject = async_class.new
         subject.executor = executor
         subject.async.echo(:foo)
       end
 
       it 'raises an exception if #executor= is called after initialization complete' do
         executor = ImmediateExecutor.new
-        subject = async_class.create
+        subject = async_class.new
         subject.async.echo(:foo)
         expect {
           subject.executor = executor
@@ -209,7 +198,7 @@ def many(*args, &block) nil; end
       it 'runs the future on the memoized executor' do
         executor = ImmediateExecutor.new
         expect(executor).to receive(:post).with(any_args)
-        subject = async_class.create
+        subject = async_class.new
         subject.executor = executor
         subject.async.echo(:foo)
       end
@@ -351,7 +340,7 @@ def gather(seconds, first, *rest)
             (@bucket ||= []).concat([first])
             @bucket.concat(rest)
           end
-        }.create
+        }.new
 
         object.async.gather(0.5, :a, :b)
         object.await.gather(0, :c, :d)