namespace everything #64

Merged
merged 1 commit into from Aug 8, 2012

Projects

None yet

2 participants

@grosser
Collaborator
grosser commented Aug 4, 2012

now that we have tests -> move everything in a place that makes more sense, splitting up main/cli/hooks,
still lots of 'extract-method' to do but this is a first rough step

@grosser
Collaborator
grosser commented Aug 4, 2012

I chose to make everything static methods and wrap the into class << self so private works as expected

@jstorimer
Owner

I said back when the project started (#5 (comment)) that I didn't want to split out the code into a bunch of files in lib/. I still believe that the code base is small enough to keep it all in bin/spin but I want to open this up for discussion and see what others think.

I personally don't see much gain from this. Having everything refactored into classes with class methods makes the code harder to find and harder to follow the path of execution. Keeping everything in one file means less overhead for grokking the source and, ultimately, for contributing. You say this 'makes more sense' but I don't see the gain.

What does everyone else think? Does this make the code more maintainable? Understandable? Easier to debug?

/cc @dstrelau @bittersweet @maprihoda

@grosser
Collaborator
grosser commented Aug 6, 2012

Hmm I'd say

pro:

  • unit-testable
  • potentially reusable by someone else
  • encourages information hiding/small methods instead of 30+ line methods with 7 random arguments
  • possible to stub/hack/overwrite via hooks
  • smaller / easily digestible chunks

con:

  • harder to follow execution because of indirection
  • harder to see what's going on
  • less instantly hackable

I kind of just feel dirty dealing with a bunch of global/unreusable methods.
A path I could also imagine is keeping options parsing in the bin/spin and putting everything else into one lib/spin.rb

@jstorimer jstorimer merged commit 095c4a6 into jstorimer:master Aug 8, 2012
@jstorimer
Owner

I'm on board :)

I am in favour of keeping option parsing in bin/spin but that can be refactored later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment