tags:

views:

52

answers:

3

I'm looking to write a Tag class (a tag is a string with no spaces). My first thought is to inherit from String:

class Tag < String
  def initialize(str)
    raise ArgumentError if str =~ /\s/
    super
  end
end

Does this look correct? Are there any other methods I should redefine?

+2  A: 

Are other letters ok?

you might want to ensure the string matches your criteria instead of not matching it.

Examples:

1. /^\w+$/      ensures at least one word character (ThisIsAValidTag_123)
2. /^[a-z]+$/   ensures at least one lowercase a to z only
3. /^[a-z]+$/i  ensures at least one a to z upper *or* lowercase.

Usage in class:

class Tag < String
  def initialize(str)
    raise ArgumentError unless str =~ /^[a-z]+$/
    super
  end
end

All that said, this sounds a lot like Ruby symbols.

macek
+1 For mentioning Ruby's Symbols; they seem to do the very thing this would be used for and more efficiently at that. I would also add `freeze` to `initialize` to prevent later changing the tag into something incompatible.
Arkku
@Arkku: You can still put spaces in symbols by putting a quote immediately after the colon. `:'hello world'`
Mark Rushakoff
@Mark: Didn't remember that syntax, thanks for pointing it out.
Arkku
You can also simply `intern` an arbitrary string to get a symbol containing any characters you want. The fact that symbols usually don't contain spaces or punctuation is just an artifact of Ruby's syntax making it awkward to write, not anything inherent in symbols.
Chuck
+1  A: 

It will be quite a challenge to make it air tight. Currently:

puts Tag.new("with_space").tr!("_"," ") # ==> prints "with space"

You would basically need to specialize all mutating methods (like #tr!), calling super and adding a check at the end, and all methods returning a modified copy of self too (like #tr).

Marc-André Lafortune
+2  A: 

You shouldn't inherit from String, both for reasons of good object-oriented design and pure pragmatism.

If you inherit from String, you are violating the Liskov Substitution Principle, which states that instances of subclasses should be substitutable for instances of their superclass. This is not the case here: I can insert a space in the middle of a String, but I can not insert a space in the middle of a Tag, therefore a Tag is not a substitue for a String, and thus shouldn't be a subclass.

And as a purely practical matter: you are inheriting roughly 100 public instance methods from String. Do you really want to audit (and potentially override) every single one of them to ensure they don't violate Tag's contract?

I would rather do something like this:

require 'facets/multiton'

class Tag
  include Multiton

  attr_reader :name
  attr_accessor :description

  private

  def initialize name, description=nil
    raise ArgumentError, 'Tag name cannot contain whitespace' if str =~ /\s/

    self.name = name.to_s.dup.freeze
    self.description = description unless description.nil?
  end

  attr_writer :name

  def self.multiton_id name, description=nil
    return name.to_s.downcase
  end
end
Jörg W Mittag