1

So I have defined this class File inside a module and what I want to do is rewrite the self.parse method so that it uses case. I'm new to Ruby so this is not straightforward for me. Also the method must contain in it's body no more than 8 lines of code. Any ideas how to do it? Also I asked it on Code Review and they said it was off topic for them.


module RBFS
    class File
        attr_accessor :content
        def initialize (data = nil)
            @content = data
        end

        def data_type
            case @content
                when NilClass then :nil
                when String then :string
                when TrueClass , FalseClass then :boolean
                when Float , Integer then :number 
                when Symbol then :symbol                                                              
            end
        end

       def serialize
           case @content
               when NilClass then "nil:"
               when String then "string:#{@content}"
               when TrueClass , FalseClass then "boolean:#{@content}"
               when Symbol then "symbol:#{@content}"
               when Integer , Float then "number:#{@content}"        
           end
       end

       def self.parse (str)
           arr = str.partition(':')
           if arr[0] == "nil" then return File.new(nil) end
           if arr[0] == "string" then return File.new(arr[2].to_s) end
           if (arr[0] == "boolean" && arr[2].to_s == 'true') then return File.new(true) end
           if (arr[0] == "boolean" && arr[2].to_s == 'false') then return File.new(false) end
           if arr[0] == "symbol" then return File.new(arr[2].to_sym) end
           return File.new(arr[2].to_i) if (arr[0] == "number" && arr[2].to_s.include?('.') == false)
           return File.new(arr[2].to_f) if (arr[0] == "number" && arr[2].to_s.include?('.') == true)
       end
   end
end

Example how 'RBFS::File.parse' works:

RBFS::File.parse("string:"Hello world") => #<RBFS::File:0x1c45098 @content="Hello world"> #Tested in irb
Noah Huppert
  • 4,028
  • 6
  • 36
  • 58
ama
  • 43
  • 5

3 Answers3

3

I would personally prefer this:

def self.parse(arg)
  key, value = arg.to_s.split(':')
  {
    'nil'     => new(nil), 
    'string'  => new(value),
    'boolean' => new(value == 'true'),
    'symbol'  => new(value.to_sym),
    'number'  => new(value.include?('.') ? BigDecimal(value) : Integer(value))
  }[key]
end

Code above is actually of 2 lines, broken into multiple lines for readability sake. However, if using case is a must then you can change your code to this:

def self.parse(arg)
  key, value = arg.to_s.split(':')
  case key
  when 'nil' then new(nil)
  when 'string' then new(value)
  when 'boolean' then new(value == 'true')
  when 'symbol' then new(value.to_sym)
  when 'number' then new(value.include?('.') ? BigDecimal(value) : Integer(value))
  end
end
Surya
  • 15,703
  • 3
  • 51
  • 74
  • What is your view on changing the last `case` to `new(case key; when 'nil' then nil; when 'string then value...end)` or `arg = case key; when 'nil' then nil;....end; new(arg)`.? – Cary Swoveland Nov 27 '14 at 06:33
  • I thought of doing that too, but, it wasn't looking good so I didn't write it, do you think it'd be a good idea to mention that as well? :) – Surya Nov 27 '14 at 06:54
  • @CarySwoveland: Two reasons - 1) `BigDecimal` is much reliable than `Float` in monetary calculations. 2) `BigDecimal` can be easily changed to float(`to_f`) but not the other way around. – Surya Nov 28 '14 at 06:11
  • Sorry, Surya, `BigNum` on the brain. Deleted my earlier comment and will delete this one too... – Cary Swoveland Nov 28 '14 at 06:19
2

In Ruby, case statements test using the case equality method #===. #=== returns true for several different of comparisons beyond the type checking you've already implemented in #serialize and #data_type. For example:

Integer === 1    //=> true
Numeric === 1    //=> true
(1..10) === 1    //=> true
1 === 1          //=> true

With that knowledge, we can construct a #parse method as follows:

def parse(serialized)
  type, content = serialized.split(':') # A neat trick that'll make things more readable.
  case type
  when 'nil'
     # ...
  when 'string'
     # ...
  when 'boolean'
     # ...
  when 'symbol'
     # ...
  when 'number'
     # ...
  else
    raise "Invalid RBFS file."
  end
end

I'm not sure that you'll be able to do this in 8 lines without compromising the file's readability or dropping the error handling step I added at the end (which I highly recommend), but to get close, you can use the when ... then ... syntax.

Community
  • 1
  • 1
fny
  • 31,255
  • 16
  • 96
  • 127
1

Below is a suggestion for one way you might write this. It is untested, of course. I made two assumptions:

  • what you refer to as a[2] is a string, so a[2].to_s is unnecessary.
  • if a[0] => "boolean", a[2] is 'true' or 'false'.
module RBFS
  class File
    attr_accessor :content
    def initialize (data = nil)
      @content = data
    end

    def data_type
      class_to_sym[@content.class]
    end

    def serialize
      return nil if @content.class == NilClass
      "#{class_to_sym[@content.class].to_s}:#{@content}"
    end

    def self.parse (str)
      type,_,val = str.partition(':')
      File.new(type_to_arg(type, val))
    end

    private

    def class_to_sym
       { NilClass=>:nil, String=>:string, TrueClass=>:boolean,
         FalseClass=>:boolean, Numeric=>:number, Symbol=>:symbol }
    end

    def type_to_arg(type, val)
      case type
        when "nil"     then nil
        when "string"  then val
        when "boolean" then val == 'true' 
        when "symbol"  then val.to_sym 
        when "numeric" then val[/\./] ? val.to_f : val.to_i 
        end
      end
    end

  end
end

If you prefer, you could replace val[/\./] with val.include?('.').

You could alternatively use a hash to simulate a case statement in type_to_arg:

def type_to_arg(type, val)
  { "nil"    =>nil,
    "string" =>val,
    "boolean"=>(val=='true'),
    "symbol" =>val.to_sym,
    "number" =>val[/\./] ? val.to_f : val.to_i
  }[type]
end
Cary Swoveland
  • 106,649
  • 6
  • 63
  • 100
  • What exactly you meant by: *Note that `99.class => Fixnum`, not `Integer`*? – Surya Nov 28 '14 at 06:12
  • It seems I messed up again, but thanks. I replaced `Fixnum=>:number, Float=>:number` in my hash `Class_to_Sym` with just `Float=>:number`. That should work OK. Correct? Let me know of any other problems you spot. – Cary Swoveland Nov 28 '14 at 06:34
  • Looks good to me. Just one thing, maybe it's more of a coding style preference, so I'd say neglect it if you don't like the idea. I guess `Class_to_Sym` can be private method instead of a constant, reason is a constant will be initialized and occupies memory, while method will be initializing that hash till its scope. Also, a constant can be altered in code, while a method can not be altered. I remember in one answer you'd moved a calculative code to a method, so I thought why not move `case type ..` to a class method and then call: `File.new(determine_type type )`? – Surya Nov 28 '14 at 09:01
  • @Suyra. thanks for the advice. I've implemented both suggestions. – Cary Swoveland Nov 30 '14 at 06:22