Embracing Simplicity to Kill Conditional Complexity – A Real World Example
In the Agile India Submission system, we had a feature which would accept different url path to show matching proposals.
For example:
http://present.agileindia.org/agile-india-2014/agile-lifecycle/45_mins –> will return all 45 mins proposals under the Agile-Lifecycle track for the Agile India 2014 Conference.
http://present.agileindia.org/agile-india-2014/beyond-agile/90_mins –> will return all 90 mins proposals under the Beyond-Agile track for the Agile India 2014 Conference.
If we remove the last part .i.e. http://present.agileindia.org/agile-india-2014/agile-lifecycle –> will return all proposals under the Agile-Lifecycle track for the Agile India 2014 Conference.
and http://present.agileindia.org/agile-india-2014 –> will return all proposals for the Agile India 2014 Conference (could be other conferences as well.)
To achieve this, we had the following routes defined:
//URL Path = /agile-india-2014 app\get("/{conference}", function($req) { $query_params = structure_request_params($req['matches']); ... }); //URL Path = /agile-india-2014/agile-lifecycle app\get("/{conference}/{track}", function($req) { $query_params = structure_request_params($req['matches']); ... }); //URL Path = /agile-india-2014/agile-lifecycle/45_mins app\get("/{conference}/{track}/{label}", function($req) { $query_params = structure_request_params($req['matches']); ... }); |
In our database we had the following:
CREATE TABLE `conference` ( `key` VARCHAR(255) NOT NULL, ... PRIMARY KEY (`key`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; CREATE TABLE `track` ( `conference` VARCHAR(255) NOT NULL, `key` VARCHAR(255) NOT NULL, ... PRIMARY KEY (`key`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; CREATE TABLE `label` ( `conference` VARCHAR(255) NOT NULL, `track` VARCHAR(255) NOT NULL, `key` VARCHAR(255) NOT NULL, ... PRIMARY KEY (`conference`,`track`,`key`) ) ENGINE= InnoDB DEFAULT CHARSET=utf8; INSERT INTO `conference` VALUES ('agile-india-2014', ...); INSERT INTO `track` VALUES ('agile-india-2014', 'agile-lifecycle', ...); INSERT INTO `label` VALUES ('agile-india-2014', 'agile-lifecycle', '45_mins', ...); |
And some really complicated logic:
// For example URL Path /agile-india-2014 will result in // $req_params = array('conference'=>'agile-india-2014') // And URL Path /agile-india-2014/agile-lifecycle will result in // $req_params = array('conference'=>'agile-india-2014', 'track'=>'agile-lifecycle') // And URL Path /agile-india-2014/agile-lifecycle/45_mins will result in // $req_params = array('conference'=>'agile-india-2014', 'track'=>'agile-lifecycle', 'label'=>'45_mins') |
function structure_request_params($req_params){ $query_params = array(); if(isset($req_params['label'])){ if(Conference::exists($req_params['conference'])){ $query_params['conference'] = $req_params['conference']; if(Track::exists(array('conference' => $req_params['conference'], 'key' => $req_params['track']))){ $query_params['track'] = $req_params['track']; if(Label::exists(array( 'conference' => $req_params['conference'], 'track' => $req_params['track'], 'key' => $req_params['label']) )) { $query_params['label'] = $req_params['label']; } else{ $query_params['search_term'] = array($req_params['label']); } } else{ $query_params['search_term'] = array($req_params['track'], $req_params['label']); } } else{ $query_params['search_term'] = array($req_params['conference'], $req_params['track'], $req_params['label']); } } else{ if(isset($req_params['track'])){ if(Conference::exists($req_params['conference'])){ $query_params['conference'] = $req_params['conference']; if(Track::exists(array('conference' => $req_params['conference'], 'key' => $req_params['track']))){ $query_params['track'] = $req_params['track']; } else{ $query_params['search_term'] = array($req_params['track']); } } else{ if(Track::exists(array('key' => $req_params['conference']))){ $query_params['track'] = $req_params['conference']; if(Label::exists(array('track' => $req_params['conference'], 'key' => $req_params['track']))){ $query_params['label'] = $req_params['track']; } else{ $query_params['search_term'] = array($req_params['track']); } } else{ $query_params['search_term'] = array($req_params['conference'], $req_params['track']); } } } else{ if(isset($req_params['conference'])){ if(Conference::exists($req_params['conference'])){ $query_params['conference'] = $req_params['conference']; } elseif(Track::exists(array('key' => $req_params['conference']))){ $query_params['track'] = $req_params['conference']; } elseif(Label::exists(array('key' => $req_params['conference']))){ $query_params['label'] = $req_params['conference']; } else{ $query_params['search_term'] = array($req_params['conference']); } } } } return $query_params; } |
And supporting model classes:
class Conference{ static function exists($conference){ DB::query('SELECT * FROM conference WHERE conference.key = %s', $conference); return (DB::count() > 0); } } class Track{ static function exists($query_params){ $search_query = Track::create_search_query($query_params); DB::query("SELECT * FROM track WHERE $search_query"); return (DB::count() > 0); } private static function create_search_query($query_params){ $search_query = array(); foreach($query_params as $key => $value){ $search_query[] = "$key='$value'"; } return join(' AND ', $search_query); } } class Label{ static function exists($query_params){ $search_query = Label::create_search_query($query_params); DB::query("SELECT * FROM label WHERE $search_query"); return (DB::count() > 0); } private static function create_search_query($query_params){ $search_query = array(); if(!empty($query_params)){ foreach($query_params as $key => $value){ $search_query[] = "$key='$value'"; } return join(' AND ', $search_query); } return ''; } } |
This was extremely hard to understand, so I decided to clean it up and get rid of the conditional complexity code smell.
Refactored code:
function structure_request_params($req_params){ $criteria = array('conference', 'track', 'label'); $params = array_values($req_params); return build_search_query_params($criteria, $params, array()); } function build_search_query_params($criteria, $params, $results) { if(empty($params)) return $results; if(empty($criteria)) { if(!empty($params)) { $results['search_term'] = $params; } return $results; } if(is_present_in_db($criteria[0], $params[0])) { $results[$criteria[0]] = $params[0]; array_shift($params); } array_shift($criteria); return map_criteria_params($criteria, $params, $results); } function is_present_in_db($table_name, $value){ return (DB::queryFirstField("SELECT count(*) FROM $table_name WHERE `key` = %s", $value) > 0); } |
Also with this, we were able to delete the Model classes as they were not required.