skoba / openehr-ruby

Ruby implementation of the openEHR specification
http://openehr.jp/projects/ref-impl-ruby/
Apache License 2.0
35 stars 12 forks source link

Change loading schema. #17

Closed dem closed 11 years ago

dem commented 11 years ago
  1. For local files use require_relative instead of require. For files from other gems still use require. require_relative safer than require, because it don't go through global paths. So we can safely use monkey patching in our internal project.
  2. Change hierarhy loading to load all file on top level. Use require_relative in 3 top files: openehr.rb, rm.rb and am.rb. So we can delete require at the end of some files. For example, in item_structure.rb, content.rb.
  3. Use require_relative in head of code for dependecies between files.
  4. Delete modification of $:, unnecessary include, use long names of classes.
  5. Delete files (not top level) with only require, for example support.rb

This makes code more clean and readable.

skoba commented 11 years ago

Thank you! I am much looking forward to meeting this pull request, while I watch your commits on GitHub. Which is better, require or require_relative seems much controversy on blogs about Ruby I found. However, keeping $LOAD_PATH simple will be reduce bugs, because openEHR library has redundant name space, such as 'OpenEHR::AM::Archetype::Archetype', 'OpenEHR::RM::DataStructures::History::History'.

Спасибо!

dem commented 11 years ago

For us require_relative in openehr-ruby works better then require. We bump into 2 bugs with require. One bug appeared after bundler update, when order of LOAD_PATH was changed in strange way. Another appeared when we reload some classes in RM with monkey patching from local gems, and this broke loading of openehr-ruby gem.

I have one argument to use require. Full path to file looks more informative then relative:

require 'openehr/rm/common/archetyped'
require_relative '../common/archetyped'

If we want full path we may change it to

require_relative '../../../openehr/rm/common/archetyped'

or revert to require :-)

What is the best?

ありがとう

skoba commented 11 years ago

Other way to manage LOAD_PATH would be use File.expand_path(File.dirname(FILE)) as an idiom, such as:

  module OpenEHR
    PATH=File.expand_path(File.dirname(__FILE__))
  end

  module OpenEHR
      module RM
         require File.join(PATH, 'common')
      end
  end

For readability, your using require_relative seems better for me. Thanks!