Recipes for fixing Architecture Violations

architecture_violations.md

Recipes for Fixing Architecture Violations

This purpose of this document is to provide a guide for refactoring architecture violations.

Given

# packwerk.yml
architecture_layers:
  - admin
  - product_services
  - technical_services
  - utilities

# packs/product_services/payroll/package.yml
layer: product_services

# packs/utilities/useful/package.yml
layer: utilities

Class Method Architecture Violation

# packs/product_services/payroll/app/public/payroll/foo.rb
class Payroll::Foo
  def self.bar(name)
    "bar_#{name}"
  end
end

# packs/utilities/useful/app/public/useful/something.rb
class Useful::Something
  def do_something
    Payroll::Foo.bar("none") # architecture violation
  end
end

Here we can see that Something is calling a class method on Payroll::Foo which is in a higher layer. This is an architecture violation.

Strategies to fix the architecture violation:

1. How is Useful::Something.new.do_something used? Is it possible to move Payroll::Foo.bar to the callers of Useful::Something.new.do_something?
   1. This probably doesn't make sense if Useful::Something.new.do_something is used in many places.
2. Can Foo be moved to a lower layer?
3. DI Payroll::Foo.bar as a Proc into packs/utilities/useful
   1. Example:

module Useful::Dependencies
  def self.foo_bar=(foo_bar)
    @foo_bar = foo_bar
  end

  def self.call_foo_bar(name)
    @foo_bar.call(name)
  end
end

class Useful::Something
  def do_something
    Dependencies.call_foo_bar("none")
  end
end

# packs/product_services/payroll/config/initializers/inject_foobar.rb
Useful::Something.foo_bar = -> (name) { Payroll::Foo.bar(name) }

• Advantages
  • The Architecture violation is fixed
• Disadvantages
  • Are we hiding a bad design and making it more difficult to understand/follow the code?
  • Is this how the code would look if it was designed correctly from the start?

ActiveRecord Architecture Violation

# packs/product_services/authentication/app/models/user.rb
class User < ApplicationRecord

end

# packs/utilities/useful/app/public/useful/something.rb
class Useful::Something
  attr_reader :user_id
  
  def initialize(user_id)
    @user_id = user_id
  end
  
  delegate :email, :name, to: :user, prefix: true
  
  def do_something
    # some important stuff
    build_something_requiring_email(user_email) 
    # more important stuff
    build_something_requiring_name(user_name)
  end
  
  private 
  def user
    @user ||= User.find(user_id) # architecture/privacy violation
  end
end

Strategies to fix the architecture violation: "1" and "2"" are the same as above. 3. The DI approach used in the previous example doesn't scale

  module Useful::Dependencies
    def self.email_from_user_id=(proc)
      @email_from_user_id = proc
    end
  
    def self.call_email_from_user_id(name)
      @email_from_user_id.call(name)
    end

    def self.name_from_user_id=(proc)
      @name_from_user_id = proc
    end

    def self.call_email_from_user_id(name)
      @name_from_user_id.call(name)
    end
    
    def self.some_other_dependency=(proc)
      @some_other_dependency = proc
    end
    
    def self.call_some_other_dependency(name)
      @some_other_dependency.call(name)
    end
  end

4. DI an interface into Useful::Something

  module UsefulDependenciesInterface
    extend T::Sig
    extend T::Helpers
    
    interface!
    
    sig { returns(String) }
    def email; end
    
    sig { returns(String) }
    def name; end
  end

  module Useful::Dependencies
    def self.useful_dependencies=(dependencies)
      @dependencies = dependencies
    end
  
    def self.useful_dependencies
      @dependencies
    end
  end
  
  class Useful::Something
    def do_something
      # some important stuff
      build_something_requiring_email(Dependencies.useful_dependencies.email) 
      # more important stuff
      build_something_requiring_name(Dependencies.useful_dependencies.name)
    end
  end

• Advantages
  • Architecture violation is fixed
  • Collecting all the dependencies in an interface makes it easier to see what the dependencies are