Publisher: Extract all duplicated methods to a base class

[roback]

Was trying to fix #71, and noticed we had to do the same thing in two places, so I extracted the methods to a base class instead.

I don't remember if there was the reason to why we have two similar classes though.

[walro]

I don't remember if there was the reason to why we have two similar classes though.

Don't think so. Nice find :)

However, I think one could instantiate a BasePublisher now and it would actually do something? Not sure if that is desirable? Maybe a design based on modules could work out better

[roback]

However, I think one could instantiate a BasePublisher now and it would actually do something? Not sure if that is desirable? Maybe a design based on modules could work out better

I just thought of the BasePublished as an implementation detail that no one else should use. I'm not quite sure what happens if someone uses it actually. Maby it's possible to use private_constant, but I don't think it'll work in this case.

Haven't even considered using modules, but it's worth a try.

[walro]

I just thought of the BasePublished as an implementation detail that no one else should use.

Hehe yes, I just came to think about it. In other languages you can mark the class as abstract to avoid that.

[roback]

Maby it's possible to use private_constant, but I don't think it'll work in this case.

Apparently it does work :)

twingly-amqp mattias$ bundle console
irb(main):001:0> Twingly::AMQP::BasePublisher
  NameError (private constant Twingly::AMQP::BasePublisher referenced)

Don't think we have to add it though.

[roback]

There's not much difference between using a module and a base class. Don't think one is better than the other, so I think we can go either way.

Diff

```diff diff --git a/lib/twingly/amqp/base_publisher.rb b/lib/twingly/amqp/base_publisher.rb index 018fb13..cfe1225 100644 --- a/lib/twingly/amqp/base_publisher.rb +++ b/lib/twingly/amqp/base_publisher.rb @@ -4,11 +4,7 @@ require "ostruct" module Twingly module AMQP - class BasePublisher - def initialize(connection: nil) - @connection = connection || Connection.instance - end - + module BasePublisher def publish(message) raise ArgumentError unless message.respond_to?(:to_h) @@ -31,7 +27,7 @@ module Twingly yield options end - protected + private def options @options ||= diff --git a/lib/twingly/amqp/default_exchange_publisher.rb b/lib/twingly/amqp/default_exchange_publisher.rb index 1ec96ba..ef3235c 100644 --- a/lib/twingly/amqp/default_exchange_publisher.rb +++ b/lib/twingly/amqp/default_exchange_publisher.rb @@ -2,13 +2,14 @@ require "twingly/amqp/base_publisher" module Twingly module AMQP - class DefaultExchangePublisher < BasePublisher - def initialize(queue_name:, connection: nil) - super(connection: connection) + class DefaultExchangePublisher + include BasePublisher + def initialize(queue_name:, connection: nil) options.routing_key = queue_name - @exchange = @connection.create_channel.default_exchange + connection ||= Connection.instance + @exchange = connection.create_channel.default_exchange end end end diff --git a/lib/twingly/amqp/topic_exchange_publisher.rb b/lib/twingly/amqp/topic_exchange_publisher.rb index ca4d819..055e5a4 100644 --- a/lib/twingly/amqp/topic_exchange_publisher.rb +++ b/lib/twingly/amqp/topic_exchange_publisher.rb @@ -2,13 +2,14 @@ require "twingly/amqp/base_publisher" module Twingly module AMQP - class TopicExchangePublisher < BasePublisher - def initialize(exchange_name:, routing_key: nil, connection: nil, opts: {}) - super(connection: connection) + class TopicExchangePublisher + include BasePublisher + def initialize(exchange_name:, routing_key: nil, connection: nil, opts: {}) options.routing_key = routing_key - @exchange = @connection.create_channel.topic(exchange_name, opts) + connection ||= Connection.instance + @exchange = connection.create_channel.topic(exchange_name, opts) end end end ```

[walro]

I kind of like it, if module is renamed BasePublisher -> Publisher

[roback]

Sure, lets go with a module then