Tozier and I began the morning by talking about what we might do. There are two major features still missing. One is that our system needs to FTP up the category folders as well as the article folders. The plan is just to upload all the categories rather than try to do an analysis to find out which ones are changed. We could optimize that later.

The other bit of functionality is to set it all to run on a file watcher. That’ll be a scripting thing, most likely, rather than a Ruby thing. Maybe it’s possible in Ruby: we can look into that later if we want to.

Both of these features are “just work”, I believe. I believe there are no surprises lying there, but that they just involve applying knowledge we already have.

There’s interfacing to do. The folder names and such are rather tightly built into the code, and sometimes into the tests. Before we can run the thing live, we’ll need a better way to send in all that information. We’ve noticed that that’s a problem and we’re also uncomfortable with the strange way we have to fiddle the chdir all around.

Netting it all out, we decide to do some refactoring to get things ready for production.

Passing in a connected FTP object

We noticed that while we create JekyllRunner with a connected FTP object, the code creates and uses its own. That’s odd. So we cleaned that up and wound up here:

require "minitest/autorun"
require "./jekyllserver"

class Test_JekyllRunner  < Minitest::Test

  def setup
    @test_startup_folder = Dir.pwd
  end

  def teardown
    Dir.chdir(@test_startup_folder)
  end

  def set_up_for_jekyll_testing
    FileUtils.rm_rf("./_target")
    FileUtils.mkdir("./_target")
    FileUtils.mkdir("./_target/articles")
    FileUtils.rm_rf("./test_jekyll_site/articles")
    ipad_folder = '/Users/ron/Dropbox/_source'
    jekyll_folder = './test_jekyll_site'
    return JekyllRunner.new(ipad_folder, jekyll_folder)
  end
  
  def test_file_copy
    jr = set_up_for_jekyll_testing
    assert(!File.exist?('./test_jekyll_site/articles/a.txt'), "mistakenly found a.txt")
    jr.move_ipad_files # sorry, Demeter
    # this file down here is in jekyll input
    assert(File.exist?('./test_jekyll_site/articles/a.txt'), "can't find a.txt")
  end

  def test_jekyll_run
    jr = set_up_for_jekyll_testing
    jr.move_ipad_files # sorry, Demeter
    jr.run_jekyll # this is the big deal right here
    # these files down here is in jekyll output
    assert(File.exist?('./test_jekyll_site/_site/articles/a.txt'), "can't find jekyllated a.txt")
    assert(File.exist?('./test_jekyll_site/_site/articles/subfolder/index.html'), "can't find jekyllated subfolder")
  end

  def test_we_know_what_to_ftp
    jr = set_up_for_jekyll_testing
    jr.move_ipad_files # sorry, Demeter
    # jr.run_jekyll 
    expected_to_ftp = ["a.txt", "anotherfolder", 
      "anotherfolder/f5.txt", "anotherfolder/index.html", 
      "b.txt", "pic.JPG", "subfolder", "subfolder/index.html", 
      "subfolder/sf1.txt", "subfolder/st2.txt"]
    assert_equal(expected_to_ftp, jr.what_to_ftp)
  end

  def test_what_will_be_ftped
    jr = set_up_for_jekyll_testing
    jr.move_ipad_files
    list_of_allegedly_moved_things = jr.ftp_the_files(simulated = true) 
    assert list_of_allegedly_moved_things.include? "/Users/ron/programming/test-ftp/test_jekyll_site/_site/articles/subfolder/index.html to something/subfolder/index.html"
  end

  def test_end_to_end
    jr = set_up_for_jekyll_testing
    jr.run
    assert(File.exist?('./_target/articles/anotherfolder/f5.txt'), 
      "end to end can't find ftped f5.txt from #{Dir.pwd}")  
  end

  def test_setup_teardown_restores_chdir
    result = (`pwd`).chomp
    assert_equal(@test_startup_folder, result)    
  end
    
  def test_chdir_affects_where_backtick_runs
    Dir.chdir('/Users/ron')
    result = (`pwd`).chomp
    assert_equal('/Users/ron', result)
  end
end

require "net/ftp"
require "./passwords"

class JekyllRunner
  def initialize(ipad_folder, jekyll_folder)
    @ipad = ipad_folder
    @jekyll_folder = jekyll_folder
  end

  def ftp_connected_to_target
    ftp = Net::FTP.new('localhost')
    ftp.login(Passwords::USER,Passwords::PASSWORD)
    ftp.chdir('programming/test-ftp/_target/articles')
    ftp
  end

  def move_ipad_files
    FileUtils.cp_r("#{@ipad}/.","#{@jekyll_folder}/articles")
  end

  def ftp_mkdir_safely(ftp, folder_string)
    ftp.mkdir(folder_string) unless ftp_folder_exists?(ftp, folder_string)
  end

  def ftp_folder_exists?(ftp, folder_string)
    split = folder_string.split("/")
    proposed_folder = split.last
    ppl = split[0...-1]
    proposed_prefix = "./" + ppl.join("/")
    ftp.list(proposed_prefix).any? { |name| name.match(proposed_folder) }
  end

  def what_to_ftp
    pwd = Dir.pwd
    Dir.chdir(@ipad)
    files = Dir.glob('**/*')
    Dir.chdir(pwd)
    files.collect { |f| rename(f) }
  end

  def ftp_the_files(simulated = false)
    pwd = Dir.pwd
    Dir.chdir("#{@jekyll_folder}/_site/articles")
    # position Ftp
    ftp_connection = ftp_connected_to_target unless simulated
    result = []
    files = what_to_ftp
    files.each do |file|
      result << ftp_one_file(ftp_connection, file, simulated)
    end
    Dir.chdir(pwd)
    result
  end

  def ftp_one_file(ftp, file_name, simulated)
    unless simulated
      if File::directory? file_name
        ftp_mkdir_safely(ftp, file_name)
      else
        ftp.putbinaryfile(file_name,file_name)
      end
    end
    pwd = simulated ? "something" : ftp.pwd
    return "#{Dir.pwd}/#{file_name} to #{pwd}/#{file_name}"
  end

  def rename(file_name)
    file_name.gsub(/\.md$/,".html")
  end

  def run_jekyll
    pwd = Dir.pwd # TODO fix this to be more reasonable
    Dir.chdir(@jekyll_folder)
    `jekyll build`
    Dir.chdir(pwd)
  end

  def run
    move_ipad_files
    run_jekyll
    ftp_the_files
  end
end

We’ve acknowledged that the JekyllRunner will create its own FTP object, since it already does. Let’s see if we can pass in the host and folder info that it uses:

  def ftp_connected_to_target
    ftp = Net::FTP.new('localhost')
    ftp.login(Passwords::USER,Passwords::PASSWORD)
    ftp.chdir('programming/test-ftp/_target/articles')
    ftp
  end

We begin by creating default parameters that we could pass in:

  def ftp_connected_to_target(
      ftp_host_name='localhost', 
      ftp_folder='programming/test-ftp/_target/articles')
    ftp = Net::FTP.new(ftp_host_name)
    ftp.login(Passwords::USER,Passwords::PASSWORD)
    ftp.chdir(ftp_folder)
    ftp
  end

Getting warmer, now we think we could create the JekyllRunner with a couple more parameters, and we’d use different ones for the tests and for the real thing.

We’re not sure what to do about the passwords, as our basic design is that they’re compiled in. This keeps them sort of secret, we thought. For example, unless I let Bill explore my folders, he can’t find my real FTP passwords.

Our next step is to pass these values in from the initialize, and then to use them in the real call.

Looking around we see some issues around the use of the FTP. We can’t just create one in the initialize because it’s currently created in the ftp_the_files method:

  def ftp_the_files(simulated = false)
    pwd = Dir.pwd
    Dir.chdir("#{@jekyll_folder}/_site/articles")
    # position Ftp
    ftp_connection = ftp_connected_to_target unless simulated
    result = []
    files = what_to_ftp
    files.each do |file|
      result << ftp_one_file(ftp_connection, file, simulated)
    end
    Dir.chdir(pwd)
    result
  end

We’re doing that weird simulated trick. We also notice that we’re not closing the FTP at all. Not as clever as it might be.[^cleanup]

[^cleanup]We all see issues like this all the time. We open a file, work on it, then have to remember to close it. We set a folder position, do some work, then have to remember to set the folder position back. We open a database, do a bunch of operations, then commit or roll back. We’re going to solve our directory problems of this kind, and maybe we need to do this one as well.

Also this method needs cleaning up. Composed method, I think.

We decide that we’ll save the host and folder names as member variables: they’re exactly the kind of thing this object can use, because they live and die with the same timing.

Having done that, we don’t like it. Then we decide, instead of passing in the parameters to ftp_connected_to_target, we’ll save the strings in member variables, just as we already do for the ipad and jekyll folders. For now, we’ll just init them in the class and worry about making them parameters on the constructor later.1 Our new approach is this:

  def initialize(ipad_folder, jekyll_folder)
    @ipad = ipad_folder
    @jekyll_folder = jekyll_folder
    @ftp_host_name='localhost'
    @ftp_folder='programming/test-ftp/_target/articles'
  end

  def ftp_connected_to_target
    ftp = Net::FTP.new(@ftp_host_name)
    ftp.login(Passwords::USER,Passwords::PASSWORD)
    ftp.chdir(@ftp_folder)
    ftp
  end

Now let’s look back at that overly complicated ftp_the_files method:

  def ftp_the_files(simulated = false)
    pwd = Dir.pwd
    Dir.chdir("#{@jekyll_folder}/_site/articles")
    # position Ftp
    ftp_connection = ftp_connected_to_target unless simulated
    result = []
    files = what_to_ftp
    files.each do |file|
      result << ftp_one_file(ftp_connection, file, simulated)
    end
    Dir.chdir(pwd)
    result
  end

Since we’re passing the simulated flag into the moving code, we could unconditionally create and close an FTP if we wanted. But if we have the flag maybe we should use it. We don’t consider this an efficiency issue but a safety one. When we get closer to running this on the real folder, we can use the simulated flag to ensure we don’t send things to the real web site until we’re very sure everything works. First, let’s at least close the connection, conditioning that call on the flag:

  def ftp_the_files(simulated = false)
    pwd = Dir.pwd
    Dir.chdir("#{@jekyll_folder}/_site/articles")
    # position Ftp
    ftp_connection = ftp_connected_to_target unless simulated
    result = []
    files = what_to_ftp
    files.each do |file|
      result << ftp_one_file(ftp_connection, file, simulated)
    end
    ftp_connection.close unless simulated
    Dir.chdir(pwd)
    result
  end

OK that’s closed. Let’s refactor this baby. (Also, that list of stuff that we return in result is hideous and useful only for testing. One thing at a time though.)

The method does a few things. It saves and restores the Dir working folder; it opens an FTP connection; it gets a list of files and folders; it moves the files while saving ugly string information; it closes the connection; it restores the working folder. I lied when I said “few”.

We’ll extract some methods, to begin with:

  def ftp_the_files(simulated = false)
    pwd = Dir.pwd
    Dir.chdir("#{@jekyll_folder}/_site/articles")
    result = ftp_with_Dir_set(simulated)
    Dir.chdir(pwd)
    return result
  end

  def ftp_with_Dir_set(simulated)
    # position Ftp
    ftp_connection = ftp_connected_to_target unless simulated
    result = []
    files = what_to_ftp
    files.each do |file|
      result << ftp_one_file(ftp_connection, file, simulated)
    end
    ftp_connection.close unless simulated
    return result
  end

This code still irritated me because Dir is this massive global variable and I’d rather pass a folder into the ftp_with_Dir_set function. I think I see a better way to do it, setting the desired folder inside the called method instead of outside:

  def ftp_the_files(simulated = false)
    pwd = Dir.pwd
    result = ftp_with_Dir_protected(simulated)
    Dir.chdir(pwd)
    return result
  end

  def ftp_with_Dir_protected(simulated)
    Dir.chdir("#{@jekyll_folder}/_site/articles")
    ftp_connection = ftp_connected_to_target unless simulated
    result = []
    files = what_to_ftp
    files.each do |file|
      result << ftp_one_file(ftp_connection, file, simulated)
    end
    ftp_connection.close unless simulated
    return result
  end

We hate these names but I’m reluctant to change the top-level name until the structure is right.

I’m still troubled by this folder protection trick. On the one hand, the “idea” is “save Dir status, do my thing, restore Dir status”. On the other, all these methods are looking at a massive system-level global, the Dir class. I wish it were a parameter I could send in. Anyway I’ve no better idea right now. We have two things going, at least. One is the names, the other is we’re in the middle of cleaning up what is now the ftp_with_Dir_protected method. Fix names now and then refactor more? Or vice versa?

Tozier offers this code, using collect instead of explicitly filling the result:

  def ftp_with_Dir_protected(simulated)
    Dir.chdir("#{@jekyll_folder}/_site/articles")
    ftp_connection = ftp_connected_to_target unless simulated
    result = what_to_ftp.collect do |file|
      ftp_one_file(ftp_connection, file, simulated)
    end
    ftp_connection.close unless simulated
    return result
  end

This gets rid of some ugly « stuff. It troubles me that it masks the fact that ftp_one_file returns a result as well as FTPing one file, but I’m going to allow it because it’s shorter. Tozier changes the what_to_ftp line to look like a function call with ():

  def ftp_with_Dir_protected(simulated)
    Dir.chdir("#{@jekyll_folder}/_site/articles")
    ftp_connection = ftp_connected_to_target unless simulated
    result = what_to_ftp().collect do |file|
      ftp_one_file(ftp_connection, file, simulated)
    end
    ftp_connection.close unless simulated
    return result
  end

This better indicates that what_to_ftp is a function. We’re still creeped out by it. What does that function do anyway?

  def what_to_ftp
    pwd = Dir.pwd
    Dir.chdir(@ipad)
    files = Dir.glob('**/*')
    Dir.chdir(pwd)
    files.collect { |f| rename(f) }
  end

Arrgh another of these damn folder-saving things. And Tozier doesn’t like the rename name. It goes like this:

  def rename(file_name)
    file_name.gsub(/\.md$/,".html")
  end

This is what causes us to FTP an html file when the input file was md. We’ll give that a better name. (But I feel like we’re recurring when we should be cleaning up at the top. Note, though, as long as one makes improvements with every step, it’s probably OK, as long as the tests run. Scary, though.)2

We call it rename_md_to_html. Seems better. Tests run.

I don’t like the way those Dir.pwd things look. I want to write this:

  def ftp_the_files(simulated = false)
    return perform_with_safe_Dir { ftp_with_Dir_protected(simulated)}
  end

So I write what I want, by intention. Now I just have to make it work. No, I’m wrong. We have to make it work:

  def perform_with_safe_Dir
    pwd = Dir.pwd
    result = yield
    Dir.chdir(pwd)
    return result
  end

Tozier feels the yield is a bit obscure so we use the other form:

  def perform_with_safe_Dir(&block)
    pwd = Dir.pwd
    result = block.call
    Dir.chdir(pwd)
    return result
  end

Yeah, that’s better. We both think yield is for coroutines, which we only understand on alternate Wednesdays.

Now we have another place to use this:

  def what_to_ftp
    pwd = Dir.pwd
    Dir.chdir(@ipad)
    files = Dir.glob('**/*')
    Dir.chdir(pwd)
    files.collect { |f| rename_md_to_html(f) }
  end

This turns, first, into this:

  def what_to_ftp
    perform_with_safe_Dir do
      Dir.chdir(@ipad)
      files = Dir.glob('**/*')
      files.collect { |f| rename_md_to_html(f) }
    end
  end

And then, into this:

  def what_to_ftp
    perform_with_safe_Dir do
      Dir.chdir(@ipad)
      Dir.glob('**/*').collect { |f| rename_md_to_html(f) }
    end
  end

This is actually getting quite habitable. Did you even notice it happening? We didn’t until we got here. We both find it “remarkably satisfying” when this sort of thing happens.

I have another idea, though. Everyone who calls this function plans to change to another folder. Can’t we pass it in as the parameter? Some work and a bit of renaming to perform_in_folder.

  def perform_in_folder(new_dir, &block)
    pwd = Dir.pwd
    Dir.chdir(new_dir)
    result = block.call
    Dir.chdir(pwd)
    return result
  end

  def what_to_ftp
    perform_in_folder(@ipad) do
      Dir.glob('**/*').collect { |f| rename_md_to_html(f) }
    end
  end

  def ftp_the_files(simulated = false)
    return perform_in_folder("#{@jekyll_folder}/_site/articles") do 
      ftp_with_Dir_protected(simulated)
    end
  end

Now there’s this ugly remaining bit:

  def ftp_the_files(simulated = false)
    return perform_in_folder("#{@jekyll_folder}/_site/articles") do 
      ftp_with_Dir_protected(simulated)
    end
  end

  def ftp_with_Dir_protected(simulated)
    ftp_connection = ftp_connected_to_target unless simulated
    result = what_to_ftp().collect do |file|
      ftp_one_file(ftp_connection, file, simulated)
    end
    ftp_connection.close unless simulated
    return result
  end

Tozier proposes making the top function worse by moving all that bottom stuff back up into it. Excellent! Now that we have this nice protection code, we can look at the still-too-long block and deal with it in situ.

We tried the obvious step of moving that code from ftp_with_Dir_protected inside ftp_the_files and a test fails, saying f5.txt wasn’t moved. We put things back and it works. We must look further.

This is weird. With the code as above, we conclude that JekyllRunner as a whole is not correctly restoring the working folder on Dir. I propose, first, a hack:

  def test_end_to_end
    pwd = Dir.pwd # hack save
    jr = set_up_for_jekyll_testing
    jr.run
    Dir.chdir(pwd) # hack restore
    assert(File.exist?('./_target/articles/anotherfolder/f5.txt'), 
      "end to end can't find ftped f5.txt from #{Dir.pwd}")  
  end

We protect the test by saving pwd here and it works. This confirms our theory that someone is stepping on the Dir settings. Now we’ll remove this hack and make the test run without it. (We hope.)

Tozier asserts that if we encapsulate run this bug will go away. We have:

  def run
    move_ipad_files
    run_jekyll
    ftp_the_files
  end

We save and restore pwd in run and again the test works. But we think that should be unnecessary. We give up and do some puts:

  def test_end_to_end
    puts "In #{Dir.pwd}"
    jr = set_up_for_jekyll_testing
    puts "Mid #{Dir.pwd}"
    jr.run
    puts "Out #{Dir.pwd}"
    assert(File.exist?('./_target/articles/anotherfolder/f5.txt'), 
      "end to end can't find ftped f5.txt from #{Dir.pwd}")  
  end

And sure enough the folder changes:

In /Users/ron/programming/test-ftp
Mid /Users/ron/programming/test-ftp
Out /Users/ron/programming/test-ftp/test_jekyll_site/_site/articles

Protecting will work but we think it’s covering a bug. More puts:

  def run
    # perform_in_folder(Dir.pwd) do
      puts "1 #{Dir.pwd}"
      move_ipad_files
      puts "2 #{Dir.pwd}"
      run_jekyll
      puts "3 #{Dir.pwd}"
      ftp_the_files
      puts "4 #{Dir.pwd}"
    # end
  end

And:

1 /Users/ron/programming/test-ftp
2 /Users/ron/programming/test-ftp
3 /Users/ron/programming/test-ftp
4 /Users/ron/programming/test-ftp/test_jekyll_site/_site/articles

So that means that ftp_the_files is dirtying the folder setting. We would have sworn it wasn’t:

  def ftp_the_files(simulated = false)
    perform_in_folder("#{@jekyll_folder}/_site/articles") do 
      ftp_connection = ftp_connected_to_target unless simulated
      result = what_to_ftp().collect do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
      return result
    end
  end

I speculate that the problem is that return result. I think it’s busting out past the perform’s recovery somehow. My theory is that the return, being inside the function, has exited the perform.

We screw around a bit and find that removing the return but leaving result makes it run:

  def ftp_the_files(simulated = false)
    perform_in_folder("#{@jekyll_folder}/_site/articles") do 
      ftp_connection = ftp_connected_to_target unless simulated
      result = what_to_ftp().collect do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
      result
    end
  end

Tozier thinks this is a Ruby proc/lambda thing. We don’t understand those, we know they are weird and we plan to back away slowly.

Ron “explains” that when we had return there, it broke out before our perform_in_folder could do its restore. Without result there, the block returns the result of the close, which is nil. With result there, the block returns, well, result and the function returns it because that’s what functions do: return the last damn thing they’ve noticed.

We feel that we understand this well enough to move on and we’re sure this issue will bite us sometime in the future but probably not in this code.

We have one more explicit save/restore to fix:

  def run_jekyll
    perform_in_folder(@jekyll_folder) {`jekyll build`}
  end

This, by the way, is what Ruby should look like: a one-line method. It’s almost as nice as Smalltalk, where you just keep calling one line methods until the problem dissolves into a solution.

We’re past our time box so it’s time to stop. The tests are running. The code is better but still not perfect. Here it is:

require "minitest/autorun"
require "./jekyllserver"

class Test_JekyllRunner  < Minitest::Test

  def setup
    @test_startup_folder = Dir.pwd
  end

  def teardown
    Dir.chdir(@test_startup_folder)
  end

  def set_up_for_jekyll_testing
    FileUtils.rm_rf("./_target")
    FileUtils.mkdir("./_target")
    FileUtils.mkdir("./_target/articles")
    FileUtils.rm_rf("./test_jekyll_site/articles")
    ipad_folder = '/Users/ron/Dropbox/_source'
    jekyll_folder = './test_jekyll_site'
    return JekyllRunner.new(ipad_folder, jekyll_folder)
  end
  
  def test_file_copy
    jr = set_up_for_jekyll_testing
    assert(!File.exist?('./test_jekyll_site/articles/a.txt'), "mistakenly found a.txt")
    jr.move_ipad_files # sorry, Demeter
    # this file down here is in jekyll input
    assert(File.exist?('./test_jekyll_site/articles/a.txt'), "can't find a.txt")
  end

  def test_jekyll_run
    jr = set_up_for_jekyll_testing
    jr.move_ipad_files # sorry, Demeter
    jr.run_jekyll # this is the big deal right here
    # these files down here is in jekyll output
    assert(File.exist?('./test_jekyll_site/_site/articles/a.txt'), "can't find jekyllated a.txt")
    assert(File.exist?('./test_jekyll_site/_site/articles/subfolder/index.html'), "can't find jekyllated subfolder")
  end

  def test_we_know_what_to_ftp
    jr = set_up_for_jekyll_testing
    jr.move_ipad_files # sorry, Demeter
    # jr.run_jekyll 
    expected_to_ftp = ["a.txt", "anotherfolder", 
      "anotherfolder/f5.txt", "anotherfolder/index.html", 
      "b.txt", "pic.JPG", "subfolder", "subfolder/index.html", 
      "subfolder/sf1.txt", "subfolder/st2.txt"]
    assert_equal(expected_to_ftp, jr.what_to_ftp)
  end

  def test_what_will_be_ftped
    jr = set_up_for_jekyll_testing
    jr.move_ipad_files
    list_of_allegedly_moved_things = jr.ftp_the_files(simulated = true) 
    assert list_of_allegedly_moved_things.include? "/Users/ron/programming/test-ftp/test_jekyll_site/_site/articles/subfolder/index.html to something/subfolder/index.html"
  end

  def test_end_to_end
    jr = set_up_for_jekyll_testing
    jr.run
    assert(File.exist?('./_target/articles/anotherfolder/f5.txt'), 
      "end to end can't find ftped f5.txt from #{Dir.pwd}")  
  end

  def test_setup_teardown_restores_chdir
    result = (`pwd`).chomp
    assert_equal(@test_startup_folder, result)    
  end
    
  def test_chdir_affects_where_backtick_runs
    Dir.chdir('/Users/ron')
    result = (`pwd`).chomp
    assert_equal('/Users/ron', result)
  end
end

require "net/ftp"
require "./passwords"

class JekyllRunner
  def initialize(ipad_folder, jekyll_folder)
    @ipad = ipad_folder
    @jekyll_folder = jekyll_folder
    @ftp_host_name='localhost'
    @ftp_folder='programming/test-ftp/_target/articles'
  end

  def ftp_connected_to_target
    ftp = Net::FTP.new(@ftp_host_name)
    ftp.login(Passwords::USER,Passwords::PASSWORD)
    ftp.chdir(@ftp_folder)
    ftp
  end

  def move_ipad_files
    FileUtils.cp_r("#{@ipad}/.","#{@jekyll_folder}/articles")
  end

  def ftp_mkdir_safely(ftp, folder_string)
    ftp.mkdir(folder_string) unless ftp_folder_exists?(ftp, folder_string)
  end

  def ftp_folder_exists?(ftp, folder_string)
    split = folder_string.split("/")
    proposed_folder = split.last
    ppl = split[0...-1]
    proposed_prefix = "./" + ppl.join("/")
    ftp.list(proposed_prefix).any? { |name| name.match(proposed_folder) }
  end

  def perform_in_folder(new_dir, &block)
    pwd = Dir.pwd
    Dir.chdir(new_dir)
    result = block.call
    Dir.chdir(pwd)
    return result
  end

  def what_to_ftp
    perform_in_folder(@ipad) do
      Dir.glob('**/*').collect { |f| rename_md_to_html(f) }
    end
  end

  def ftp_the_files(simulated = false)
    perform_in_folder("#{@jekyll_folder}/_site/articles") do 
      ftp_connection = ftp_connected_to_target unless simulated
      result = what_to_ftp().collect do |file|
        ftp_one_file(ftp_connection, file, simulated)
      end
      ftp_connection.close unless simulated
      result
    end
  end

  def ftp_one_file(ftp, file_name, simulated)
    unless simulated
      if File::directory? file_name
        ftp_mkdir_safely(ftp, file_name)
      else
        ftp.putbinaryfile(file_name,file_name)
      end
    end
    pwd = simulated ? "something" : ftp.pwd
    return "#{Dir.pwd}/#{file_name} to #{pwd}/#{file_name}"
  end

  def rename_md_to_html(file_name)
    file_name.gsub(/\.md$/,".html")
  end

  def run_jekyll
    perform_in_folder(@jekyll_folder) {`jekyll build`}
  end

  def run
    move_ipad_files
    run_jekyll
    ftp_the_files
  end
end

Summing up

I just have one question. What did we set out to do, and did we do it? OK, a compound question. Let’s reflect a moment.

We did a couple of small clean-ups and then decided to improve that messy method. That’s pretty much what we did, except that we followed our noses, and our occasionally breaking tests, to find things to clean up.

This seems odd to me, but in a good way. We thought, more or less, that we knew what we’d clean up. Instead, we started there and wandered around the campground, picking up trash wherever we noticed it. The campground is cleaner now but not quite as we might have predicted. (Tozier says “We cleaned up some legacy code that we discovered.” I choose to forget that we wrote most of that legacy code a few days ago.)

Is it good that we cleaned up this code? The result sure is better. But should we feel badly that we thought we were going to improve the interface between tests and JekyllRunner class but in fact we just cleaned up JekyllRunner? You get to decide for yourself.

We feel good about this session. We could look at this code and know, at least better than three hours ago, what it does. Nice. Worth it? I think so because I’d rather deal with these problems while I vaguely remember what the program is about.

But “of course” we could perhaps have done another feature. There are only two or three more features to do so maybe we could have jammed them in and then got the hell out while the getting’s good.

Maybe. My story is that habitable code is always faster to work on, and you’re not going to talk me out of it. What should you do? Whatever you think is best.

No, really. When I program, it’s rarely if ever on deadline, which gives me the luxury of pushing down to as tiny a step as I wish, or working without a net, or whatever strikes me. I like to see what happens, and over the years it has been clear to me that things actually go better when I work test-driven and keep my code as habitable as possible. I believe that will work for you, but you need to take your time to experiment as you see fit, and to make your own decisions.

That’s how life should be, I figure.

Late update

I tweeted for the name of the pattern that wraps a block with an open/close kind of thing. It seems likely that “Execute Around Method” is the official Kent Beck name, though I think my copy of Smalltalk Best Practice Patterns is in a box somewhere.

Along with that came a few people mentioning the Ruby ensure keyword, which would, well, ensure that the close is done. Using that would probably have prevented the problem we chased that revolved around that inner return statement.

We’ll put that in next time. Thanks for tuning in!

  1. Do these seem like tiny steps to you? Would you have gone further in one bite? Well, on a given day I might as well. Arguably a lot of the code we have here is what I get if I go a long way. I find it useful, often, to go in ludicrously tiny steps, because I find that it doesn’t really slow me down and often I find better ways part way through. Had I gone the whole way, I often feel like the improvement seems too far away. Sunk cost syndrome, I guess. ↩

  2. We’re drilling deeper and deeper here, when we thought we were working on the interface between the tests and the JekyllRunner. This can often be an indication of a problem. “When you discover you’re in a hole, stop digging.” But our tests are kept running right along, so we figure improving the code anywhere is a good thing. ↩