push.cx
Peter Bhat Harkins
Software Craftsmanship Tour: Aidan Rogers
A few weeks ago I met up with Aidan Rogers to hack on some code. Aidan and I were coworkers at Cambrian House a few years ago.
Aidan and I hacked away at adding a feature to a Web App That Shall Not Be Named. User authentication was handled by Facebook or Twitter, so the feature was to fetch and list their friendships from their social network. The code in app/models/user.rb was straightforward, taking an object from authentication:
# Update the database with the user’s current list of friends. # Compares a fresh list to the existing list to minimize database writes. def update_friends(auth) # fetch current list of friendships as hashes provider_friendships = auth.retrieve_friendships # load already-stored friendships to hashes stored_friendships = friendships.send(“on_#{auth.provider}”).map { |friend| {:id => friend.friend_id, :name => friend.friend_name} }
# compare the two lists of hashes
# add any new friends to the database (provider_friendships - stored_friendships).each do |friend| friendships.create!({ :friend_id => friend[:id], :friend_name => friend[:name], :provider => auth.provider, }) end
# delete any removed friends from the database (stored_friendships - provider_friendships).each do |friend| friendships.destroy_all(:conditions => { :friend_id => = friend[:id], :friend_name => friend[:name] }) end end
That code isn’t very complicated, but it’s worth noting before you read the next snipped that we were both new to the Facebook/Twitter APIs and spiking functionality rather than trying to write production quality code.
Keep that in mind as you read the code we added to the authentication object. It works, but it could definitely be better written (left as an exercise to the reader, because, well, we only spiked, so I haven’t actually written a better version to present).
| ` def retrieve_friendships if provider == “facebook” # made the Facebook API call user = FbGraph::User.me(self.data[‘credentials’][‘token’]) # map it to an array of hashes return user.friends.map do | friend | # check against the db to load their id existing = Authentication.where(:uid => friend.identifier, = :provider => ‘facebook’).first { :friend => existing ? existing.user : nil, :provider_id => existing ? nil : friend.identifier, :provider => ‘facebook’ } end elsif provider == “twitter” # Returns only 20 users at a time - is it important enough at this = point to add all the code for iterating? (aidan) # return Twitter.friends(nickname).users.map{ | friend | Hash[ :id => = friend.id, :name => friend.screen_name, :provider => ‘twitter’ ]} return {} else raise “Refactor me” end end `{lang=”ruby”} |
Having the authentication object test what provider API it should be loading from is an obvious code smell, but it’s not the one I want to talk about.
There’s an implicit type in use. Aidan and I referred to it as a FriendHash, it’s a normalization for comparing friendship objects from the app’s database and from different API calls. FriendHash would be a data class with just a smidge of validation code to account for all three attributes.
I’ve noticed these sneaking around code in the interfaces between objects. I don’t quite have a name for them, but I realize it touches a bit on design by contract. The part that smells to me is that this code is spread amongst many places that need to be updated in sync with each other (not that this is terribly better if you have a class for it, but at least you can grep for references to it). I’m not a big fan of data classes, so I don’t love this solution.
What really frustrates me is that I’ve got enough experience to have a tingling intuition when I am attempting to re-solve some problem that surely someone else has analyzed and offered solutions to, but my resesarch hasn’t turned anything up.
So I’m posting to hear what folks think of this situation. Have you read anyone writing about these quiet but complex data types at interfaces, or do you have ideas for how to address them?