aws / aws-sdk-ruby-record

Official repository for the aws-record gem, an abstraction for Amazon DynamoDB.
Apache License 2.0
318 stars 41 forks source link

How to inherit common attributes from a base class? #80

Closed awendt closed 1 year ago

awendt commented 6 years ago

I have 2 models that share a number of attributes, but have some of their own. Is there any way I can use this library to do this?

If I put the include on the super class:

class Animal
  include Aws::Record
  string_attr :name
  integer_attr :age
end

class Dog < Animal
  boolean_attr :family_friendly
end

class Cat < Animal
  integer_attr :num_of_wiskers
end

Then either Dog.new or Cat.new raises:

NoMethodError: undefined method `register_attribute' for nil:NilClass

If I move include Aws::Record into the subclasses, loading Animal stops working.

If I include Aws::Record in all 3 classes, the subclasses don't inherit attributes because of the way Aws::Record::Attributes.included works.

I can make this work using:

class Animal
  def self.attributes
    { name: 'string', age: 'integer'}
  end
end

class Cat < Animal
  include Aws::Record
  Animal.attributes.each_pair do |name, type|
    send("#{type}_attr", name)
  end
  integer_attr :num_of_wiskers
end

class Dog < Animal
  include Aws::Record
  Animal.attributes.each_pair do |name, type|
    send("#{type}_attr", name)
  end
  boolean_attr :family_friendly
end

but doing this feels like having to work around the library's limitations. Is there a better way?

awood45 commented 6 years ago

This is something that hasn't worked, it's been a bit tricky to get right in my first attempts.

xcskier56 commented 5 years ago

@awendt, I got this working with a module and composition instead of inheritance.

module Animal
  def self.included(base)
    base.include Aws::Record
    base.set_table_name 'animal_table_name'

    base.string_attr :primary_key, hash_key: true
    base.string_attr :sort_key, range_key: true

    base.datetime_attr :created_at
    base.datetime_attr :updated_at
  end
end

# and then use in your models

class Cat
  include Animal

  integer_attr :num_of_wiskers
end

class Dog
  include Animal

  boolean_attr :family_friendly
end

It might not be the inheritance that you're looking for, and so for making hirearchical classes it might not work great, but personally, I think multiple levels of inheritance is a, code smell so this might 🤞 lead to better design

jorihardman commented 3 years ago

@awood45 I agree with @awendt that the current inheritance behavior runs counter to what you would expect from rails class conventions. It's possible to change by making the @attributes class instance variable an ActiveSupport class_attribute which is inheritable by subclasses. This would likely be a major version change, and having not dug too deeply into the source, there may be other class instance variables that would need the same treatment for attribute inheritance to work as expected.

Is there any interest in pursuing that route?