3

I have a helper class defined as follows:

require "toml"

module Test
  class Utils
    @@config

    def self.config
      if @@config.is_a?(Nil)
        raw_config = File.read("/usr/local/test/config.toml")
        @@config = TOML.parse(raw_config)
      end
      @@config
    end
  end
end

When I call this method elsewhere in the code:

server = TCPServer.new("localhost", Utils.config["port"])

I receive the following compile-time error:

in src/test/daemon.cr:10: undefined method '[]' for Nil (compile-time type is (Hash(String, TOML::Type) | Nil))

      server = TCPServer.new("localhost", Utils.config["port"])

There is no way for Utils.config to run something that is Nil, so I don't understand the error.

  1. How do I tell the compiler Utils.config will always return something that is not Nil?
  2. (Minor additional question) Is this a good design pattern for a resource (the config) that will be shared between classes, but only should be created once?
James Taylor
  • 6,158
  • 8
  • 48
  • 74

2 Answers2

4

The problem with your code is that while checking if @config is nil (this is btw. easier with @config.nil?) in the if branch, the value of that instance variable might have changed until it reaches the return line. The compiler has to assume it can be nil again if it was changed from a different fiber for example.

You can save it to a local variable instead and return this

class Utils
  @@config

  def self.config
    if (config = @@config).nil?
      raw_config = File.read("/usr/local/test/config.toml")
      @@config = TOML.parse(raw_config)
    else
      config
    end
  end
end

Or a bit refactored but essentially the same thing:

class Utils
  @@config

  def self.config
    @@config ||= begin
      raw_config = File.read("/usr/local/test/config.toml")
      TOML.parse(raw_config)
    end
  end
end

I'd prefer setting @@config to nil over the default initialization with an empty object because it clearly signals that this object is not available. If the config file happened to be empty for example, the check for empty? would always trigger a reload and parsing, hence eliminating the memoization feature.

The ||= operator basically means

if config = @@config
  config
else
  @@config = # ...
end
Johannes Müller
  • 5,581
  • 1
  • 11
  • 25
  • Looks good. I didn't think crystal's compiler could infer when using `if something.nil?` - either I'm mistaken or they added that feature. – marksiemers Nov 03 '17 at 22:33
1

EDIT: See Johannes Müller's answer below, it is a better solution.


In general, if you want to avoid Nil, you should type your class and instance variables:

@@config : Hash(String,Toml::Type)

That will help the compiler help you - by finding code paths that could lead to a Nil value and alerting you during compile time.

A potential fix for the code:

require "toml"

module Test
  class Utils
    @@config = {} of String => TOML::Type # This line prevents union with Nil

    def self.config
      if @@config.empty?
        raw_config = File.read("/usr/local/test/config.toml")
        @@config = TOML.parse(raw_config)
      else
        @@config
      end
    end

  end
end

puts Test::Utils.config["port"]

I couldn't test that directly due to the toml requirement, but a running example using strings instead is here: https://play.crystal-lang.org/#/r/30kl

For your second question, this approach may work:

require "toml"

module Test
  class Utils
    CONFIG = TOML.parse(File.read("/usr/local/test/config.toml"))
  end
end

puts Test::Utils::CONFIG["port"]

Example code using a string instead of TOML: https://play.crystal-lang.org/#/r/30kt

marksiemers
  • 631
  • 8
  • 14
  • I'm not sure if initializing a constant with data loaded from a file is such a good idea. The lazy-loading approach with a class variable should probably be preferred. – Johannes Müller Nov 02 '17 at 13:22