Over the last few months I have been deep in my latest VIM integration (announcement coming soon) written with 95% ruby (core) and 5% vim (wrapper). At this point in it’s development I have proved out the functionality for vim-ruby communication, key mappings, data display, CRUD operations, exception handling, logging, and most importantly, aspects of (mouse-less :) usability. Now, there is clear line of sight to start building functionality at a very rapid pace. However, I am forcing myself to stop. cold.
Stop. Think. Refactor.
Why stop? To stop and embrace the evolutionary aspect of designing the architecture. If I continue, I run the risk of building 10-20x more code on top of user paths with incomplete exception handling, lots of FIXME and TODOs, and quite frankly, various areas with fairly good but really just prototype code. The existing code is not a loss. Rather is was a series of experiments; ‘successful’ experiments. Even the failures were a success. And I refactored a lot along the way, so the current body of code is still quite valuable. But it’s time to inspect.
Example: Loading ruby classes.
I struggled a lot with LOADPATH visibility in the ruby runtime embedded inside vim. If you ‘require’ a file in your ruby class, it tried to open that class file relative to the current directory where you’re generally editing – not your vim plugin lib path. This actually turned out to be OK because I started to just ‘require’ all the files up front. But I had to register the names manually, load base classes before subclasses, and I didn’t want to load the universe of classes in case the plugin failed to load (e.g. remote service not available).
Refactor: This weekend I was surfing the ruby Kernel API and was reminded of ‘autoload’ to simply register the files to load only if the Class/Module is actually referenced. I turned about 20 lines of code, located in 2 files, with nested conditions, into the following solution that relies on the [Rails-like] convention where camel-case class names match underscore filenames:
53 # Pre-register ALL ruby files to be loaded on-demand^ 54 # convention: underscore filename => camelized module^ 55 # example: update_task_todo_controller.rb --> :UpdateTaskTodoController^ 56 rbfiles = File.join(VIM::evaluate('g:rally_lib_dir'), "ruby", "**", "*.rb")^ 57 Dir.glob(rbfiles).each do |filename_abs|^ 58 filename_noext = File.basename(filename_abs).gsub!(/\.rb$/, '') # extract name of file, no extension 59 modname = filename_noext.gsub(/(^|_)(.)/) {$2.upcase} # camelize filename^ 60 autoload(modname, filename_abs)^ 61 end^
Example: Global variables.
Yes. I know. I felt dirty at the mere thought of using a… glo… glob… global variable. But after looking at the code, almost 90% of it used the same variable that was passed around EVERYWHERE. It was the connection class to the remote service that also served as a cache… the whole plugin relies on this connection across the board. Now the initial architecture is fairly MVC… but this means that the controller grabs the connection, passes it to the model, the model passes it to utility classes. And only the handful of view classes are immune. This also meant that automated testing would require… ugh… mocking a connection and passing to all test classes.
Refactor: I created a single top-level global variable ($conn). This cleaned up SO much of the API that using the global variable actually felt… clean. Now I’m not advocating that it was the most pure approach, but from simplification of the APIs and code… it was a great!
68 $conn = ConnectionController.new.conn^
Example: Testable ruby.
In vim, if you want to envoke ruby code, you do this:
10 ruby << FOO 11 puts "bar" 12 FOO
That only makes your vimscript file get messy by having so much ruby code mixed with vimscript. So instead, you can tuck those lines above into foo.rb and within vim do:
10 runtime /path/to/foo.rb
But that’s the problem! Look at your ruby file. The file is flanked by the vim/ruby load syntax “ruby << FOO …. FOO”. So you can’t load that foo.rb file within ruby for independent… TESTING! I had a much bigger problem writing tons of ruby classes that couldn’t be tested (easily) than dealing with a single global variable.
Refactor: Turns out that the first example above solved this issue entirely. By using autoload() to preload the files, those files are written entirely in pure ruby. Solved. Now all classes can be loaded and independently tested.
Summary
I want to also add more exception handling for core activities, support for trap’ping interrupts (e.g. user hits CTRL-C during network activity), and review the MVC flows that currently require 5 files (controller, model, view, formatter, display manager).
I’ll “lose” about 2 weeks doing this architecture refactor. But I’m confident that it’ll make adding features 2-3x faster and the overall codebase will be maintainable.
