Ensure that a directory reflects a specified Git commit

Question

I have a method that looks like the method below, and I can't help but shake the feeling that it can be improved. It doesn't really "read" well, and it seems like checkout could be called only once, somehow.

def ensure_correctness
  if exist?
    unless correct_version?
      delete!
      checkout
    end
  else
    checkout
  end
end

def checkout
  run_command(['git', 'clone', '--depth', '1', url, path])
end

def exist?
  Dir.exist?(path)
end

def delete!
  FileUtils.rm_rf(path)
end

def correct_version?
  commit_sha_short == desired_commit_sha_short
end

The best I could come up with is moving the code from failing correct_version? to a new method (which probably isn't a terrible idea, but I wonder if I can do better).

I feel like I'm missing something really obvious here...

Answer 1

The two calls to checkout look well to me. They are simple method calls to self with no arguments, and I can easily see that both calls are the same. You avoid making two identical calls to run_command(['git', 'clone', ...]).

The "something really obvious" might be a return:

def ensure_correctness
  if exist?
    return if correct_version?
    delete!
  end
  checkout
end

Answer 2

While I envisioned just moving the instructions in ensure_correctness around a little, to find an efficient solution to a pattern a see repeated with some frequency, it turns out it was better to rearrange everything a little. I think the final solution is more attractive, even if it does potentially call delete! unnecessarily (it doesn't matter).

def ensure_correctness
  unless correct_version?
    delete!
    checkout
  end
end

def checkout
  run_command(['git', 'clone', '--depth', '1', url, path])
end

def exist?
  Dir.exist?(path)
end

def delete!
  FileUtils.rm_rf(path)
end

def correct_version?
  exist? && commit_sha_short == desired_commit_sha_short
end