Wednesday, September 17, 2014

Separating concerns: Data vs Algorithm

I'm by no stretch of the imagination a Knuth, but I was pairing the other day with a newer developer; and we started looking at some fairly complex code - at least from their perspective.
    key: :known_issue_ids, 
    data: Something::KnownIssue, 
    map_to: :known_issues
    key: :depth_of_market_ids, 
    data: Something::DepthOfMarket, 
    map_to: :depth_of_market
    key: :developer_history_ids, 
    data: Something::DeveloperHistory, 
    map_to: :developer_histories
].each do |param_value|
  property_params[param_value[:map_to]] = create_data_mapping(param_value[:data].all, property.send(param_value[:key])) if  property.send(param_value[:key]).any?

Obviously, there's a lot going on there; but it seemed quite impenetrable. I couldn't seem to convey that the importance of the technique was splitting the input data from the actual operation/algorithm; even if it resulted in some slightly more abstract code.

For example, it's quite often easy to look at duplicated, similar code and realise that introducing a method would greatly reduce issues; but we often fail to do this with conditionals or iterators. Languages with lambdas/procs/blocks/etc tend to make this a little bit easier to pull off; markedly so when it's translating from A to B with a lot of exceptions to rules.

Here, it becomes a lot more obvious that mapping is really just a set of lookups; compared to other code like

  b.foo_translated =
if a.foo2
  b.foo2_translated = a.foo2
if a.foo_catfish
  b.blah_translated = a.foo_catfish

The above even looks simpler for simple operations, but when you start performing really convoluted logic the chance of mistakes increases; particularly if you end up doing copy/paste/replace scutwork.
I'm curious - how wide spread is this practice?

Is it just a variation on certain DRY refactorings; or is it just an idiom I've picked up inadvertently from other languages like C?

Apart from the minor cognitive load put on the next maintainer (I should have really added a one line comment explaining what it boiled down to); are there any inherent negatives?

No comments: