Implemented Tramway Form inheritance

[kalashnikovisme]

What's changed basically?

Tramway Form inheritance implemented

What's changed for tramway drivers?

Example

Before

class UserForm < TramwayForm
  properties :email, :password
end

class AdminForm < UserForm
  properties :permissions
end

AdminForm.properties # returns [:permissions]

After

class UserForm < TramwayForm
  properties :email, :password
end

class AdminForm < UserForm
  properties :permissions
end

AdminForm.properties # returns [:email, :password, :permissions]

Playlist for the code-review (Spotify)

Find more info here

[haukot]

I see that you're using @properties ||= [] several times in the base_form.rb class. You could initializes the @properties class instance variable one time when the class is loaded

class << self
   @properties = []

  def property(attribute)
    (__ancestor_properties + @properties).uniq
  end
end

[kalashnikovisme]

I've never seen this approach. Are you sure that initializing class-level variables out of methods is nice? 🙂

[haukot]

It's for sure nicer to have one place of default initialization, and I dont see problems with class level variable.

But I also haven't used this approach much, so can't comment if it have no problems at all

[kalashnikovisme]

I've tested this approach. For some reason, @properties is not initialized on class load.

[haukot]

Ah, I see, the variable initialization doesn't pass to child classes. Sorry, then, my bad :)
The initialization still could be isolated with inherited hook, if you willing to go that path, like this

class Parent
  def self.inherited(subclass)
    subclass.instance_variable_set(:@things, [])
  end

  class << self
    def add(value)
      @things << value
    end
  end
end

class ChildDecorator < Parent
  add :third
  add :four
end

[kalashnikovisme]

Nice! 🙂 Thanks!!

btw, Linters are v ahuie 😆

[haukot]

btw, Linters are v ahuie

They are right, it should be @properties, not @@properties :) @@properties will give you effects you don't want, like - if you change child properties, the parent properties also will be changed

[kalashnikovisme]

Maybe I missed something, but looks like we can't use @properties in our case.

subclass.class_variable_set(:@properties, __ancestor_properties)

return NameError: '@properties' is not allowed as a class variable name

[haukot]

because it should be instance_variable_set, not class_variable_set. As a class is actually instance of Class itself, and class variables are @@ ones

[haukot]

@kalashnikovisme this shouldn't be needed

[kalashnikovisme]

Will fix in the next PR