Вопрос

I have the following models:

module Core
  class Conditioner
    include Mongoid::Document

    field :operator, type: String, default: '' # can be !=, ==, <, >, <=, >=, =
  end
end

module Core
  class Count < Conditioner
    field :threshold, type: Integer, default: 0 # a simple threshold
  end
end

module Core
  class Time < Conditioner
    UNITS = %w(seconds minutes hours days weeks months years)

    # will be use like this: Time.value.send(Time.unit) Ex: 3.minutes
    field :value, type: Integer, default: 0    # 3
    field :unit,  type: String,  default: ''   # minutes

    validates :unit,  presence: true, inclusion: { in: UNITS }
  end
end

Was wondering if I should namespace the Count and Time class with Conditioner ? Like this:

module Core
  class Conditioner::Time < Conditioner
  end
end

Since I have to call Time.now like this now ::Time.now.

EDIT

Regarding answers, maybe this should be a better idea:

module Core
  module Conditioner
    class Base
    end
  end
end

module Core
  module Conditioner
    class Count < Conditioner::Base
    end
  end
end

module Core
  module Conditioner
    class Time < Conditioner::Base
    end
  end
end

Since defining a class called Core::Time is maybe too generic and does not make many sense.

What do you think? Not sure about the best practice here.

Это было полезно?

Решение

You don't have to namespace it, although you can if you want to.

Whether you should or shouldn't depends on what do you want to model (and not whether you have to address Time with ::Time or not...). It looks like your Core::Time already subclasses Core::Conditioner, so it doesn't make much sense to make it an inner class of it's superclass. In your case it is better not to namespace it.

Having the same class name as ruby is not an issue here, since you have already namespaced it with Core.

Другие советы

In my opinion, Don't use a class name that Ruby or Rails has used, it may be a problem. You must be careful whenever you use it. I think it is unnecessary. Don't use namespaces unless you have many models to be maintained. Keep it simple and the maintenance will be easier :)

The only thing you change by prefixing Conditioner to Time is that the constants lookup for the Core module will not see your Conditioner::Time class when looking up Time. All the other classes (Count, Conditioner) will still think Conditioner::Time is superseding Time.

module Core
  def time
    Time.now
  end
  class Conditioner
    def time
      Time.now
    end
  end
end

module Core
  class Count < Conditioner
    def time
      Time.now
    end
  end
end

module Core
  class Conditioner::Time < Conditioner
    def time
      Time.now
    end
  end
end

Any call like Core::Count.new.time will fail, but

class A
  include Core
end

A.new.time will put out the current time, while in this case:

module Core
  def time
    Time.now
  end
  class Conditioner
    def time
      Time.now
    end
  end
end

module Core
  class Count < Conditioner
    def time
      Time.now
    end
  end
end

module Core
  class Time < Conditioner
    def time
      Time.now
    end
  end
end

class A
  include Core
end

A.new.time will also fail. Now, the reason why is fairly easy: The constants lookup always looks at the constants available in the current namespace (Core), then climbs up the ancestor chain of the current namespace before looking in the namespace of Object (Time can also be called as Object::Time. It won't search in child namespaces (which you create by putting Conditioner:: before Time) though, and that's why renaming your class Time to Conditioner::Time only changes lookup for the Core module, but not for the classes within Core.

So, asking for best practice in this case: Just leave it at calling your class Time and referencing to Object::Time by ::Time. This is a fairly well known practice, and the benefits of additional namespaces to avoid using it are marginal.

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top