I have found some common code patterns/refactoring patterns while doing code reviewing in my current day job. And one of them involves switch blocks. After reading nate’s blog here about switch blocks, I thought to share something similar.
Following is an example code, where switch-case is used to configure some variables based on the value of $api_name.
swicth($api_name) {
case 'flickr':
$class = 'Phlickr';
$subalbum = false;
$auth = 'redirect';
break;
case 'photobucket':
$class = 'PBAPI';
$subalbum = true;
$auth = 'redirect';
break;
case 'smugmug':
$class = 'PHPSmug';
$subalbum = false;
$auth = 'direct';
break;
//
//
}This is a good-looking handsome piece of code, no doubt about that. But the following is a definite smarter one,
$api_settings = array(
'flickr' => array( 'class' => 'Phlickr', 'subalbum' => false, 'auth' => 'redirect' ),
'photobucket' => array( 'class' => 'PBAPI', 'subalbum' => false, 'auth' => 'redirect' ),
'smugmug' => array( 'class' => 'PHPSmug', 'subalbum' => false, 'auth' => 'direct' ),
//
);
if(isset($api_settings[$api_key])) { extract($settings[$api_key]); }
Design wise it is more flexible - you can just add an array in the $api_settings when a new api gets added to the list, it is slicker and handy when you have quite a few apis in your list. And performance wise, certainly hashes are faster than multiple conditional statements.
Another way to avoid dumb switch-cases/conditional statements can be using the dynamic features of PHP (+ some conventions). Let’s have a look at the following code that uses switch-case to determine the method to execute,
switch($type) {
case 'users':
$count = $this->__countUsers();
break;
case 'projects':
$count = $this->__countPojects();
case 'galleries':
$count = $this->__countGalleries();
case 'tags':
$count = $this->__countTags();
default:
$count = 0;
}We can convert it to something like this,
$count = 0;
$method = '__count'.ucwords($type);
if (method_exists($this, $method)) {
$count = $this->$method();
}
We have reduced lines of code. And is not it much prettier? And now if we need to add a new type there, we can just add a method following the same convention.
Get rid of all your dumb conditionals… use PHP’s dynamic features ;)
Reader Commentary
10 peoples have commented so far.
Junal
Monday, November 17th, 2008 at 12:47amThis is very common method that our university teachers thought us. while i was a student, i wouldn't think any other way as they were teaching that this "Switch blocks" didnt have any other way to solve it.
Nice one indeed! i hope everybody follows this way instead of using dumb switch blocks .
Daniel Hofstetter
Monday, November 17th, 2008 at 1:23amI think a cleaner solution for your API example would be to have a class for each API implementing the same interface. The interface would look like:
interface ApiSettings {public function getClassName();
public function hasSubAlbum();
public function getAuth();
}
Anupom
Monday, November 17th, 2008 at 7:30am@daniel, I definitely agree with you. This example code was just for examples sake. I did not find any other example in my mind at that time :) In real I would have an interface, create bridge classes for all those 3rd party APIs. And perhaps would have a parent class for all those bridge classes to share some common functions across them, like converting PHP arrays to XML of a specific format :P
nhm tanveer hossain khan (hasan)
Monday, January 19th, 2009 at 12:21pmit reminds me our code which we added with extra smile ;)
we were trying to make our tester life easier
http://github.com/we4tech/mailtrap/tree/master (check out the project to know how ;)) .
one of our projects was using php mailer with codeigniter to send out mail.
we already enabled multi scoped configuration system on codeigniter, so to configure smtp configuration we had two options - either we change the default php mailer configuration or we keep them in scoped configuration.
since changing default configuration must have top to down impact propagation. so we thought it would have been better if phpmailer configuration came up with the layered configuration support inbuilt.
so here comes the trick we used, which is similar to your second example -
we kept an associated array in scoped configuration - ie
$config["email_configuration"] = array(
"Mailer" => "SMTP",
"Host" => "bla bla..",
# .... other configuration
)
on our email service we initialized php mailer with the extra constructor parameter.
ie. new PHPMailer($email_config)
on PHPMailer class we wrote the following code -
ie.
public function __constructor($p_options = array()) {
foreach ($p_options as $property_name => $value) {
$this->$method_name = $value;
}
}
thats how it is working, thus we enabled different configuration for phpmailer on test/production/developers environments.
though we could extend the class with new constructor or we could write the constructor code inside the factory method.
btw, how do you prevent yourself from not writing hardcoded url (haven't tried cake php before) ie.
article/url-id/">
i would prefer
$article_id)) ?>">
anyway, nice article indeed, keep your figures running over and over your keyboard.
best wishes,
Sabuj Kumar Kundu aka manchumahara
Monday, February 2nd, 2009 at 8:37amThanks anupam vai for this nice tips. Sometimes code optimization depends on language, isn't it ? as u are using extract here. BTW. how about the time of execution ?
anupom syam
Wednesday, February 11th, 2009 at 4:38am@hasan, thanks for sharing your thoughts on this. somehow your question got sanitized by this smart blog script! As far as I got, the answer of your question is - you can use regex in routes configuration.
here's a simple example,
Router::connect('/user(.*).xml',array('controller' => 'sitemaps', 'action' => 'get'),
array( 'pass' => array('id'),
array('id'=>'([A-Za-z0-9-_]+)') )
);
@manchumahara, yeah sometimes! sometimes you have to sacrifice performance for design's sake. object oriented programming is slower than structured programming in general. but we all advocate OOP most of the time :) something like that - there is always a trade off between performance and design. good engineers know how to balance :)
once I solved a very tricky problem in 4 lines using Ruby. but the performance sucked! then I rewrote it in 15 lines and it was almost 4 times faster. things like this happen all the time! I will share that piece of code someday in this blog, may be!
nhm tanveer hossain khan (hasan)
Saturday, February 21st, 2009 at 1:39pmhi,
stupid script, didn't know how to convert to > ;)
lets say on your view you write the following hard coded url -
a href="/items/323" >view this link / a >
if you have this type of so many views where u have so many hardcoded urls. i guess it would become a pain when your url convention get into a new touch.
ie . if /items/3232 changed to /user/items/323
so my question is, how you are maintaining your hardcoded urls?
or are u writing something like rails routing name > _url ie
a href=" %= items_url(3232) % >" >view this link / a > stuffs in php?
i guess i could explain my question :)
best wishes,
nanoman
Thursday, July 2nd, 2009 at 6:02amSorry man, but the second example sucks. If you're working in a team, you just don't do such things just for the sake of a few lines less code. This code is not readable - which is always more important than LOC.
afruj
Wednesday, July 22nd, 2009 at 6:08pmHi
I'm a new iPhone developer. need ur help. I have a NSInteger variable which value I set from another class. it works fine. But I can't get the value from viewDidLoad function. from this function I have to display background image dynamically based on the variable value. Please add me to your IM.
Thanks,
Afruj Jahan
mejor casino en linea
Monday, August 10th, 2009 at 7:05pmDesign wise it is more flexible - you can just add an array in the $api_settings when a new api gets added to the list, it is slicker and handy when you have quite a few apis in your list. And performance wise, certainly hashes are faster than multiple conditional statements.