thingsinabucket.com

Instantiating behaviour

Wed, 08 Jul 2015 00:00:00 +0000

In last weeks post we had a quick look at a little hack called digg.

some_complex_json_rsponse.digg('preview', 'thumbnail', 0, 'id')

In order for this to work we need to extend each class we want to digg with the digg method. This leads to modifying core classes; and we should frown upon that. 😠

So what else can we do?

module Digg
  def self.digg(soil, path)
    # …
  end
end

Digg.digg(some_complex_json_rsponse, ['preview', 'thumbnail', 0, 'id'])

That would work. I bet this will very quickly lead to a “Util” class that become a bag of tricks. 👜✨ We don’t want that.

So how about adding some instantiation?

class Digg
  def initialize(soil)
    @soil = soil
  end

  def digg(path)
    # …
  end
end

digger = Digg.new(some_complex_json_rsponse)
digger.digg(['preview', 'thumbnail', 0, 'id'])

Alright! Now we can digg for multiple paths on the same set of data. 😎

But… This doesn’t feel right either. 😧 The act of digging is defined by what steps are taken. (e.g.: Pick up spade, stab in ground, lift, dump, repeat). It is not defined by the soil that we digg in.

array_of_nested_objects.map { |json|
  Digger.new(json).digg(['preview', 'thumbnail', 0, 'id'])
  Digger.new(json).digg(['preview', 'name'])
}

See! That just doesn’t look good.

What if we turn that around?

class Digg
  def initialize(path)
    @path = path
  end

  def digg(soil)
    # …
  end
end

digger = Digg.new(['preview', 'thumbnail', 0, 'id'])
digger.digg(some_complex_json_rsponse)

Now we define “how to digg” when initializing. See how much better this looks for loops:

thumbnail_digger = Digg.new(['preview', 'thumbnail', 0, 'id'])
name_digger = Digger.new(['preview', 'name'])
array_of_nested_objects.map { |json|
  thumbnail_digger.digg(json)
  name_digger.digg(json)
}

Finally! 😌

This way of thinking about your objects, by defining behaviour on initializing, has a lot of benefits.

Not only is it faster (in this case) (gist), but also more intention revealing. 📖

By instantiating behaviour you can start to compose your objects with more ease, or start thinking about caching or optimising the behaviour.

Let me know what you think. And if you apply this in your code I’d be happy to have a look at it.

🎩

Leave a comment

Three Little Hacks

Wed, 01 Jul 2015 00:00:00 +0000

As I’ve missed my last 2 posts, I’m going to now regale you with not 1, not 2, but 3 (three!) of my favorite little hacks ruby hacks.

Digg

Do you have a deeply nested structures you want 1 value from deep inside? Don’t care if the path is broken? Use digg! I especially love this to get deeply nested data out of a JSON response.

module Digg
  def digg(*path)
    path.inject(self) do |memo, key|
      (memo.respond_to?(:[]) && (memo[key] || {})) || break
    end
  end
end

Array.send :include, Digg
Hash.send :include, Digg

# Use as
some_complex_json_rsponse.digg('preview', 'thumbnail', 0, 'id')

_{(Would you believe there is no digging related emoji? Have a 📯 instead.)}

Symbol respond_to

Sometimes we switch on classes, you’ve seen this before right?

case a_thing
  #...
when Array, Hash
  #...
when String
  #...
end

Not only is that ugly, but we are basing our behavior on classes, instead of behavior. Wouldn’t it be cool if we could test for methods? Well we can!

case a_thing
when ->(x) { x.respond_to? :fetch }
  #...
when ->(x) { x.respond_to? :split }
  #...
end

Ugh… That looks horrible 😷. Let’s improve that with:

module SymbolRespondTo
  def ~@
    -> (o) { o.respond_to?(self) }
  end
end

Symbol.send(:include, SymbolRespondTo)

Adding a simple unary method on symbol gives us a great looking switch. ✨

case a_thing
when ~:fetch
  #...
when ~:split
  #...
end

If I could love code, this would be the code I love. 💞💗💖

Better to_s

Often times it makes sense for the to_s value of an object to be the name or the title of the person or thing. So out of shear laziness 😫 I wrote this little hack.

module DefaultToS
  def to_s
    case self
    when ~:name then name
    when ~:title then title
    else super
    end
  end
end

ActiveRecord::Base.prepend(:to_s)

It might be a hack, but I like it.

Do you have your favorite hacks? What do you think of mine? Let me know below!

Leave a comment

Defaults for Hash

Wed, 10 Jun 2015 00:00:00 +0000

As a follow up to my first post I’d like to quickly run over the “default value” related features of Ruby’s Hash and explain when to use them.

Hash#default

totals = Hash.new(0)
totals[:jeff] # => 0
totals[:ann] += 1 # => 1

Use this when:

the default value is the same for every key

Do not use this when:

you are planning on mutating the value returned when the key is missing

scored = Hash.new([])
scored[:jeff] # => []
scored[:ann] << 'A+'
scored[:jeff] # => ['A+']
scored # => {}

💩

Hash#default_proc

fizz_bang = Hash.new { |hash, key| hash[key] = fizz_and_or_bang(key) }
fizz_bang[3] # => 'Fizz'
fizz_bang[5] # => 'Bang'

Use this when:

Each key might need a different value
And the value can be derived by the same method or proc

Don’t user this when:

You have a set of default values ( just merge your hashes )
You are not saving the value in the hash 💾 ( you should reconsider if you need a Hash at all )

Changing default methods

It’s possible to change the default_proc and default value. The last assignment will be what’s used.

h = Hash.new
h[:jeff] # => nil
h.default = 'default'
h[:jeff] # => 'default'
h.default_proc = -> (*) { 'default_proc' }
h[:jeff] # => 'default_proc'
h.default = nil
h[:jeff] # => nil
h # => {}

Hash#fetch

New Ruby programmers probably do something like this:

hash = {}
hash[:key] ||= 1

This works fine until some falsy values are introduced:

hash = {1 => nil, 2 => false}
hash[1] ||= 1
hash[2] ||= 2
hash # {1 => 1, 2 => 2}

😩

When using || your falsy values will be overwritten. This is most probably not your intention. Instead of that use Hash#fetch.

hash = {1 => nil, 2 => false}
hash.fetch(1) { 1 }
hash.fetch(2) { 2 }
hash # {1 => nil, 2 => false}

Hash#fetch is best used when:

The default value for individual keys are calculated in a very different way
Values can be falsy

It’s also great to ensure a key is present

Hash.new.fetch(:a_missing_key)
# KeyError: key not found: :a_missing_key

Don’t use it when:

You’d like the default values to be saved in the hash
You have a set of default values ( just merge your default into the hash )

Hash#fetch also has a second notation for the default value:

Hash.new.fetch(:a_missing_key, 'default value') # => 'default value'

I only use the notation with a block. There are no real benefits to the other notation except maybe some performance difference in super simple cases. In most cases though, the block version will be the better choice. It will postpone the evaluation of the default value until it’s needed; and - at lest to me - it looks better. 😍

Some idiosyncratic fun

So, what do you think this will do?

Hash.new('default value') { |hash, key| hash[key] = key.to_s }

How about this?

Hash.new.fetch('default value') { 'value' }

👉 Try it yourself 👈, I found it very surprising 😃

Leave a comment

Refactoring for fun and profit

Wed, 03 Jun 2015 00:00:00 +0000

Today I will guide you through a class refactoring done in one of the projects I work on. Ideally we should be doing this test driven, but - as this is for fun - I won’t today.

class VirusScanner

  attr_reader :file_path

  def initialize(file_path)
    @file_path = file_path
  end

  def infected?
    !Settings['virusscan_off'] && scan != 'OK'
  end

private

  def scan
    return @scan if @scan

    command = Cocaine::CommandLine.new('clamscan', "--no-summary :file").command(file: file_path)
    _, stdout, stderr = ::Open3.popen3(command)

    @scan = parse_clamscan_output(stdout.gets, stderr.gets)
  end

  def parse_clamscan_output(stdout, stderr)
    stderr = stderr.to_s.split("\n").reject{|line| line =~ /\ALibClamAV Warning:/ }.join("\n")
    raise RuntimeError, "The 'clamscan' command line was unable to execute. #{stderr}" if !stderr.blank?

    stdout.to_s.split("\n").last.split(":")[1].strip
  end

end

This is a simple interface around the clamscan scanner. This code has several smells. 😷

Turning off the virus scanner is done using a global setting
Several external dependancies make it more difficult to port
Train wrecks all over the place 🚂🚋
So many Bangs 💥

Our primary goal is to keep backward compatibility with the rest of the code base, while cleaning up the code.

#1 Separate the generic VirusScanner from the specific clamscan.

By doing this separation we can focus on the the VirusScanner without needing to worry about clamscan.

class VirusScanner

  attr_reader :file_path

  def initialize(file_path)
    @file_path = file_path
  end

  def infected?
    return false if Settings['virusscan_off']
    ClamScan.new(scanner).infected?
  end

end

class ClamScan

  attr_reader :file_path

  def initialize(file_path)
    @file_path = file_path
  end

  def infected?
    scan != 'OK'
  end

private

  def scan
    #...
  end

  def parse_clamscan_output(stdout, stderr)
    #...
  end

end

Cool. 😎

#2 Removing the “Settings” for the virus scanner

I want to get rid of the app specific ‘virusscan_off’ setting. Honestly I’m ashamed I ever wrote that 😳

The first step is to make the behavior of the class configurable.

class VirusScanner

  attr_reader :file_path

  def initialize(file_path, options = {})
    @file_path = file_path

    @scanner = options.fetch(:scanner) {
      ->(file_path) { ClamScan.new(scanner).infected? }
    }
  end

  def infected?
    return false if Settings['virusscan_off']

    @scanner.call(file_path)
  end

end

We created an easy way to understand the interface for VirusScanner’s scanner. It’s now also very easy to extract the ‘virusscan_off’ setting.

scanner_with_off_switch = ->(*args) {
  Settings['virusscan_off'] ? false : ClamScan.new(scanner).infected?
}

This works! However! This breaks one of our earlier requirements: “keep backward compatibility”. This solution also requires the developer to know what the default scanner is, and that a new smell; a smell I don’t like. 😡

Let’s do better!

class VirusScanner

  def self.default_scanner
    @default_scanner ||= -> (file_path) { ClamScan.new(scanner).infected? }
  end

  def self.default_scanner=(scanner)
    @default_scanner = scanner
  end

  attr_reader :file_path

  def initialize(file_path, options = {})
    @file_path = file_path
    @scanner = options.fetch(:scanner) { self.class.default_scanner }
  end

  def infected?
    @scanner.call(file_path)
  end

end

old_scanner = VirusScanner.default_scanner
VirusScanner.default_scanner = ->(*args) {
  Settings['virusscan_off'] ? false : old_scanner.call(*args)
}

Ok. I’m pretty happy with that. 😌

Now we turn to the ClamScan class.

#3 Remove external dependancies

Cocaine has a lot of powerful features, has tones of great use cases~~, and I had a phase where I was totally addicted to it~~. 😇

Here Cocaine is only used to safely use a file name in a shell command. 🐌 Shellwords - a standard lib - can deal with this easily.

class ClamScan
#...
private

  def scan
    return @scan if @scan

    command = Shellwords.join(['clamscan', '--no-summary', file_path])
    _, stdout, stderr = ::Open3.popen3(command)

    @scan = parse_clamscan_output(stdout.gets, stderr.gets)
  end

  def parse_clamscan_output(stdout, stderr)
    stderr = stderr.to_s.split("\n").reject{|line| line =~ /\ALibClamAV Warning:/ }.join("\n")
    raise RuntimeError, "The 'clamscan' command line was unable to execute. #{stderr}" if !stderr.blank?

    stdout.to_s.split("\n").last.split(":")[1].strip
  end

end

#4 Use the correct version of Open3

Open3.popen3 opens a stdin pipe, we don’t need that at all. A better choice would be Open3.capture3.

class ClamScan
#...
private

  def scan
    return @scan if @scan

    command = Shellwords.join(['clamscan', '--no-summary', file_path])
    stdout, stderr, _ = Open3.capture3(command)

    @scan = parse_clamscan_output(stdout, stderr)
  end

  def parse_clamscan_output(stdout, stderr)
    stderr = stderr.split("\n").reject{|line| line =~ /\ALibClamAV Warning:/ }.join("\n")
    raise RuntimeError, "The 'clamscan' command line was unable to execute. #{stderr}" if !stderr.blank?

    stdout.split("\n").last.split(":")[1].strip
  end

end

Let us also clean the code… 🚿

class ClamScan

  attr_reader :file_path

  def initialize(file_path)
    @file_path = file_path
  end

  def infected?
    @infected ||= begin
      stdout, stderr, status = Open3.capture3(command)
      !clamscan_result_safe?(stdout, stderr, status)
    end
  end

private

  def command
    Shellwords.join(['clamscan', '--no-summary', file_path])
  end

  def clamscan_result_safe?(stdout, stderr, status)
    raise_on_stderr(stderr)
    stdout.lines.last.split(':', 2).last.strip == 'OK'
  end

  def raise_on_stderr(stderr)
    clean_stderr = stderr.lines.reject { |line| line =~ /\ALibClamAV Warning:/ }
    return if clean_stderr.empty?

    fail RuntimeError, "The 'clamscan' command line was unable to execute. #{clean_stderr.join("\n")}"
  end

end

Doesn’t that look a lot better? We did quite a bunch of changes here.

Comparing the output of parse_clamscan_output to a string is not very descriptive, and it’s brittle. It’s better to just have a method that changes the output to the direct answer we are interested in.
Move exception handling into its own method
Prefering fail over raise for original errors
Use String#lines instead of String#split
Extract the command variable to a method
Skip unnecessary cast from Array to String and use Array#empty?

Only one more line to go. There is still a train wreck in the clamscan_result_safe? method.

def clamscan_result_safe?(stdout, stderr, status)
  raise_on_stderr(stderr)

  result_line = stdout.lines.last.strip
  scanned_file, result = result_line.split(':' 2)
  result == 'OK'
end

That’s a lot easier to understand. 🎓

This improvement also hints at an other improvement I could add: By making sure the scanned_file actually matches the @file_path given, we would be even more sure about the behavior of this class.

Final thoughts

There is still room for improvement here.

Better handling of the clamscan binary, and PATH
Extracting the clamscan output processing to it’s own object?
Does it make sense that calling a scanner returns false when infected?
You see something I missed? 🔎

Let me know if you like more posts like this. Also, let me know if you are interested to seem me do this same refactoring using TDD. Or, if you have a piece of code you’d like me to refactor; send it to me! 📩

Leave a comment

Hash and default_proc

Wed, 27 May 2015 00:00:00 +0000

One of my favourite features in Ruby is Hash#default_proc. Let’s see what it can do.

Ever seen code like this?

result = {}
some_collection.map do |some_entry|
  result[some_entry] = some_expensive_operation(some_entry)
end
result

Stop writing that ✋.

Unless all the values in the collection will be used, the expensive calculations should be delayed until it’s needed.

result = Hash.new do |hash, key|
  hash[key] = some_expensive_operation(key)
end

That looks a lot better already! 🎉

Ruby will evaluate the code in the block when a key is missing. This makes it great tool for these types of lookups, or as a quick in memory cache.

In last weeks post we saw that we can call procs using brackets. When those operations become expensive, we can just wrap it with a Hash without changing our client code.

expensive_proc = ->(v) do
  puts "Expensive calculation"
  sleep 1
  [v] * 2
end

expensive_proc_with_cache = Hash.new do |hash, key|
  hash[key] = expensive_proc[key]
end

def repeat_10_times(proc)
  10.times do |i|
    proc[ i % 2 ]
  end
end

repeat_10_times(expensive_proc) # Takes about 10 seconds
repeat_10_times(expensive_proc_with_cache) # Takes about 2 seconds

Great improvement! 👍

How about when you make an API call, and it returns the next and previous values as well, it would be a waste not to use them.

paginated_lookup = ->(index) do
  v = "index[#{index}]"
  {
    index => v,
    (index + 1) => v + '.next',
    (index - 1) => v + '.prev',
  }
end

paginated_lookup_cache = Hash.new do |hash, key|
  hash.merge! paginated_lookup[key]
  hash[key]
end

paginated_lookup_cache[10] => "index[10]"
paginated_lookup_cache[11] => "index[10].next"

We can populate the Hash based on the previous calls. Sweet! 🎂

Caveat! 🐹

The Hash#default_proc will not be called when using Hash#fetch. This surprised me. 👻

times2 = Hash.new {|_,v| v*2 } # => {}
times2.fetch(1) # KeyError: key not found: 1

Please don’t use this “workaround” either.

times2.fetch(1) { |k| times2[k] }

That’s just 💩.

Leave a comment

Procs and brackets

Wed, 20 May 2015 00:00:00 +0000

Looking things up in a hash is concise, readable and fun. :smile:

dictionary['ruby']
# => 'A precious stone'

Finding items in an array is not that much fun. :disappointed:

term = 'ruby'
(list_of_definitions.find { |definition| definition[:term] == term } || {})[:definition]
# => 'A precious stone'

We can have our delicious Hash lookup notation, for any type of lookup we want with a little proc trickery. :sparkles:

dictionary = -> (word) {
  (
    list_of_definitions.find { |definition| definition[:term] == word } || {}
  )[:definition]
}
dictionary['ruby']
# => 'A precious stone'

Ruby’s procs support the [] notation for their invocation (prc[]). Using that knowledge we can present any complex lookup with our syntactic sugar. :+1:

Leave a comment

A start of a new blog

Mon, 18 May 2015 00:00:00 +0000

Thingsinabucket.com is a new blog about programming, mostly in ruby. Every Wednesday a new post will go up.

This blog is shared as a Creative Commons Attribution-NonCommercial-ShareAlike 4.0 International License. You can find the source, on github.

If you have any questions, ideas, feedback etc, let me know either via the repo or the comment threads below.

Leave a comment