CODEWITHSUNDEEP
ruby
Andrew
Andrew

Reputation: 7883

Referencing class attributes in mixin

I'm fairly new to Ruby, and I have a code organisation problem.

I have a class, called Movie, which includes a module called IMDBMovieInfo.

class Movie

    include IMDBMovieInfo
    attr_accessor :name
    attr_accessor :year
    attr_accessor :movieID
end

IMDBMovieInfo has a method which takes the movieID and uses it to build an IMDB URL:

module IMDBMovieInfo
    def imdb_url()
        "http://www.imdb.com/title/tt#{self.movieID}/"
    end
end

The problem here is I'm not sure I should be referencing something in the movie class, as IMDBMovieInfo doesn't know about that class, and shouldn't. I could add an argument of a movie ID, but then if you don't know about the Movie object, you'd be doing this, which doesn't make sense:

movie = Movie.new("Titanic", "1997", "0120338")
movie.imdb_url(movie.movieID)

What would be the correct way to organise this code?

Upvotes: 0

Views: 341

Answers (2)

Patrick Oscity
Patrick Oscity

Reputation: 54734

You would only need to extract this code into a separate module if you were to use it in multiple classes. I can however reenact the wish to split up a large model into several small chunks.

You could for example establish a protocol that the including class must adhere to and leave it up to the including class where the id comes from. In the following example, the including class must implement the method imdb_id which is supposed to return the id that will be used for the url:

module IMDBMovieInfo
  def imdb_id
    raise NotImplementedError, 'including class needs to override imdb_id'
  end

  def imdb_url
    "http://www.imdb.com/title/tt#{imdb_id}/"
  end
end

class Movie
  include IMDBMovieInfo

  def imdb_id
    self.movieID
  end

  attr_accessor :name
  attr_accessor :year
  attr_accessor :movieID
end

# In another classs
# (suppose IMDB lists computer games in the future)
class ComputerGame
  include IMDBMovieInfo

  def imdb_id
    self.gameID
  end

  attr_accessor :name
  attr_accessor :year
  attr_accessor :gameID
end

I must however say that I find this whole extracting into mixins a bit clumsy. Another way would be to create a utility class that knows how to build an url but not where the id comes from:

class IMDBUtil
  def initialize(imdb_id)
    @imdb_id = imdb_id
  end

  def imdb_url
    "http://www.imdb.com/title/tt#{@imdb_id}/"
  end
end

class Movie
  include IMDBMovieInfo

  def imdb_url
    IMDBUtil.new(self.movieId).imdb_url
  end

  attr_accessor :name
  attr_accessor :year
  attr_accessor :movieID
end

To wrap this up, here's a great blog post from CodeClimate on how to refactor fat Rails models.

Upvotes: 2

FlyingFoX
FlyingFoX

Reputation: 3509

In respect to your comment I would say, that the IMDBMovieInfo module shouldn't have a function that needs a movieID. You could just move it to your Movie class.

Upvotes: 1

Related Questions