I want to do refactoring on my bad smell code using polymorphism. it's a removing 'if' statements issue

Question

I'm an iOS app developer. I got stuck on some refactoring issue while removing bad smells on my code. it's like this.

My project uses a XML data,

<resource>
   <item type='textbox' ... />
   <item type='checkbox' ... />
   <item type='selectbox' ... />
</resource>

and I use it by this code.

Item *item = nil;
for (Element *itemElement in resourceChilds)
{
   ...
   if ([[itemElement valueForAttributeNamed:@"type"] isEqualToString:@"textbox"])
   {
      item = [[Textbox alloc] initWithProperty:itemAttributes];
      ...
   }
   else if ([[itemElement valueForAttributeNamed:@"type"] isEqualToString:@"checkbox"])
   {
      item = [[Checkbox alloc] initWithProperty:itemAttributes];
      ...
   }
   else if ([[itemElement valueForAttributeNamed:@"type"] isEqualToString:@"selectbox"])
   {
      item = [[Selectbox alloc] initWithProperty:itemAttributes];
      ...
   }
   ...
}

'Item' class is the super class of 'Textbox', 'Checkbox' and 'Selectbox' classes.

And 'itemAttributes' object is an instance of NSDictionary.

As you can see above through 'initWithProperty:itemAttributes' I already gave the 'type' attribute value into an 'Item' instance. I think it's possible to use this information in the 'Item' instance to specialize it to Textbox or Checkbox or Selectbox.

Is there any way to remove these 'if' statements and refactoring this? Any suggestion are welcome. And I really appreciate it sharing your time for this.

Thanks,

MK

I think this could help http://stackoverflow.com/questions/1174093/create-objective-c-class-instance-by-name — anticyclope, Feb 29 '12 at 05:33
yeah, the link you gave helped me a lot like I didn't know the way to create a instance by its class name. Thanks a lot, anticyclope. — mongkoon, Mar 20 '12 at 08:51

ThomasW · Accepted Answer · 2012-02-29T06:07:39.767

@anticyclope's suggestion is helpful. You don't necessary need to get rid of the ifs, but you can make the code a lot simpler. Here are some things you should do:

Extract the code within the for loop into its own method, something like itemForItemElement:.
Create a itemClassForItemElementType: method. This method will greatly simplify the implementation of itemForItemElement:, but still uses the ifs.
Optionally update the itemClassForItemElementType: to return the class using a NSDictionary. This isn't necessary, but might be what you'd do if you want the mapping to be dynamic, lets say if you create one in an external file. Here you'd use the code suggested in the link from @anticyclope.

A chain of ifs is something to avoid if you are duplicating the logic multiple places, but if you only do it once it is not so bad.

Be sure to have unit tests for all these changes.

yes, I got understood your extract approach and a nice suggestion. It's so kind and delicate answer for me. Thanks ThomasW, it helped me a lot. :) — mongkoon, Mar 20 '12 at 09:09

I want to do refactoring on my bad smell code using polymorphism. it's a removing 'if' statements issue

1 Answers1