Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Let's say I have the following architecture:

module Keyring
    @keyring = Keyring.new
    class Keyring
        def key
        def add_key
        def update_key
        def remove_key
    class KeyStorage
    class Encryptor
        class AesEncryptor
class Connection
    def id
    def user_id
    def protocol
    def name
class FtpConnection
    include Keyring
    def host
    def port
    def username_encryption_id
    def passsword_encryption_id
    def is_passive
    def username
        # Return the decrypted value given an encryption key id.
        return @keyring.key(self.username_encryption_id)
    def password
        # Return the decrypted value given an encryption key id.
        return @keyring.key(self.password_encryption_id)

As you see, FtpConnetion uses Keyring.

Now, isn't Keyring coupled to the Keyring module (by name)? If I want to use FtpConnection in another project, I'd have to bring along Keyring or implement another module with exactly that name, right? Isn't this bad design?

share|improve this question
    
Are you missing some end statements here, or is this just some crazy class-within-class-within-class structure I've never seen before? :p –  KChaloux Aug 31 '12 at 12:24
    
It's not a complete code listing. –  Chad Johnson Aug 31 '12 at 14:48
    
Ah, I missed the word architecture in the description. Carry on, then. –  KChaloux Aug 31 '12 at 16:18
add comment

3 Answers

I think you've kind of missed the point of a mixin here.

A mixin in Ruby is usually a Module with a number of methods in (no classes) which can be included into any class, to extend its functionality.

For example, if you had

module Keyring
    def key
    def add_key
    def update_key
    def remove_key

then you could include this in FtpConnection, which would gain the four methods defined above.

Here is a good article about writing mixins.

What you're trying to do here is encapsulation. For this, you should just move the @keyring field into the FtpConnection class and remove the include.

module Keyring
    class Keyring
        def key
        def add_key
        def update_key
        def remove_key
    class KeyStorage
    class Encryptor
        class AesEncryptor
class Connection
    def id
    def user_id
    def protocol
    def name
class FtpConnection
    @keyring = Keyring.new
    def host
    def port
    def username_encryption_id
    def passsword_encryption_id
    def is_passive
    def username
        # Return the decrypted value given an encryption key id.
        return @keyring.key(self.username_encryption_id)
    def password
        # Return the decrypted value given an encryption key id.
        return @keyring.key(self.password_encryption_id)

Now this does create a tight coupling between the two classes, so you should probably pass the keyring into a constructor (dependency injection).

share|improve this answer
1  
So you would recommend I go a dependency injection route to provide classes access to the keyring instance? Isn't this anti-Ruby, considering everyone advocates modules/mixins over DI? –  Chad Johnson Jul 25 '11 at 21:53
    
@Chad: Who is this "everyone"? DI and mixins are two very different things. –  pdr Jul 25 '11 at 21:59
    
By "everyone" I'm mainly referring to bloggers and StackOverflow-ers. So which would you strive for if you were in my shoes, solving this problem: a mixin approach or a DI approach? –  Chad Johnson Jul 25 '11 at 22:12
    
@Chad: I cannot find what you're referring to, except for someone in 2006 trying (and failing) to invent mixin injection as an alternative to constructor injection or setter injection, so I can only assume you've misread. Rubyists are generally against IOC Containers, because they're entirely unnecessary bloat in Ruby. Maybe that's what you mean? For your case, I would certainly take a constructor injection approach, if I were you. –  pdr Jul 25 '11 at 23:21
add comment

If this kind of name coupling would have to be regarded as bad design, then Rails would be a very bad piece of software and we would have to give up the whole idea of inheritance. Every piece of software depends on library classes and they always have names that are a constant from the moment you include and use them. Say I write something with the Qt library, then I will have everything "coupled" to things like QCheckBox. Or if I use Rails, then I can't get around using ActiveRecord and if I would like to replace it, then I would need to rewrite the whole thing with exactly the same names for classes and methods.

You have a module with a clearly designed interface. The FtpConnection class doesn't need to know anything about the internal state of Keyring or access any of it's internal data. As long s it's in a file of it's own, it can at any time be replaced by another module with the same name and interface.

But I would reconsider the declaration and usage of the @keyring variable as a kind of global variable in the module itself. As long as there is no absolute need for several classes to share a single instance, this should be just declared independently within FtpConnection.

Edit:

If you need a single instance I would maybe create a central Application module (like the Rails module in Ruby on Rails) that holds it as a global variable in form of a class instance variable, something on the line

module App
  class << self
    def keyring
      @@keyring ||= Keyring.new
    end
  end
end

And pdr is right of course, that what you have here is not a mixin. Mixins would work in a different way. They access methods defined in the class that includes them. In some sense, since it's way of Ruby to implement something similar to multiple inheritance (but somehow more powerful) you could say, that they are a super class that accesses functionality in it's child classes, which is a big load of coupling. But as mentioned above, you shouldn't worry about coupling within inheritance structures. I think that term relates more to larger modules of a project that should stay independent or need very flexible options for replacement like database drivers. But even there you are still dependent on the names of classes and modules that actually implement this behavior.

Edit:

You would just require it in the head part of the file before the actual FtpConnection class and then access it with App.keyring.

Be careful not to confuse require and include. Require makes sure, that the module is loaded only once. Include would load it as often as you use it. This is mostly used with mixins (but as pdr said, you do not really have a mixin here)

require 'app.rb'

class FtpConnection
  def username
    return App.keyring.key(self.username_encryption_id)
  end
end

Edit:

Yes, somehow that's the case. Though, if your library, app, class or whatever uses a Hash, then it is coupled to an implementation of a Hash class. Or something similar, it must at least implement something with the same name and interface. That's just how modular software works. If you go on with your project you will most likely use Ruby's Net module to implement ftp functionality. so you are more or less forced to have a line like

require 'net/ftp'

From that moment on you are bound to have this module available or a similar one. If you write an app that uses FtpConnection, the same comes true or this class. No way out. Not really anyway, you can only have more abstraction layers, but that's no use in such a case.

Depending on your needs you can repeat the same pattern. If you reuse FtpConnection itself you must have the App module. Though you could of course do the same trick, but keep the keyring in The Keyring module itself.

Or you group all the classes that need the keyring in another module. This way you could at least have the central access in a single place. Assume you want to reuse FtpConnection, but this time use another keyring named KeySomething:

module App
  class << self
    def keyring
      @@keyring ||= KeySomething.new
    end
  end
end

Finished with one change of code. Same for any kind of initializing that the keyring may require. Though of course the new keyring has still to implement all the methods you use in your code.

share|improve this answer
    
My application needs one keyring instance for use with multiple other classes application-wide. This is similar to a database connection. Using a dependency injection-based design, I would instantiate the keyring object at the application level, store it in a registry, and access it as a service via a service locator. Thus, it is not a singleton. Does this make sense? What would you recommend I do, considering it is needed application wide? –  Chad Johnson Jul 25 '11 at 20:03
    
Thanks for the added response. So, let's say I put the keyring instance in the App module. Would I then "include App" in my FtpConnection class and access the keyring instance there via the "keyring" method which would be mixed into the FtpConnection class? Or would I access the keyring instance in FtpConnection via "App.keyring"? Or something else? –  Chad Johnson Jul 25 '11 at 21:52
    
Cool, thanks again for the follow-up of the follow-up. Now, going further, if I use App.keyring to access the keyring instance, aren't any libraries that require a keyring instance coupled to App (and require it to come along for reuse in any other Rails app utilizing keyring)? –  Chad Johnson Jul 25 '11 at 22:15
add comment

Well generally speaking, yes. However, don't worry about getting too fanatical in regards to sticking with perfect design (but in general try to). If you just have to do a little copy-pasta, that's alright. I won't tell.

share|improve this answer
    
Well, I'm comparing this to a dependency injection approach, which might be closer to perfect and would not require copying and pasting. –  Chad Johnson Jul 25 '11 at 18:51
    
Ah, I see. To be totally honest, I'm not all the familiar with Ruby. I'm a lazy C++ programmer :P. However, couldn't you store the name temporarily inside of a variable that is set on-load? That's how I usually solve similar dependency problems within UScript (if you're familiar with the language). By this, I mean a sort of "I'll shove this information in here, then I'll give it to you so you can read it." –  David Young Jul 25 '11 at 18:53
    
Maybe a symbol name could be used for the module name. Or, better yet, maybe a fancy class macro could be used in conjunction with a symbol. Hm. Does anyone know a commonly-used way around this coupling? –  Chad Johnson Jul 25 '11 at 18:58
    
Now you're thinking like a boss. I'd recommend a quick Google search my good friend. Sorry I couldn't be of significant help to you. I have benefited from this question though. Thanks. –  David Young Jul 25 '11 at 19:02
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.