Login | Register   
LinkedIn
Google+
Twitter
RSS Feed
Download our iPhone app
TODAY'S HEADLINES  |   ARTICLE ARCHIVE  |   FORUMS  |   TIP BANK
Browse DevX
Sign up for e-mail newsletters from DevX


advertisement
 

eXtreme.NET Iteration One: Refactoring with Resharper : Page 3

This article follows a team of developers learning to use XP (eXtreme Programming) techniques to improve the way they deliver software. Follow this team as they learn about Resharper, a tool that may help them refactor their code.


advertisement
Understanding the Task
Chris: Sure. We need to change it so it can account for having a sponsor pay for some of the costs. This reduces the amount that the course will cost to attendees.

Pete: Right. So we just need to add the sponsor payment to the XML document and then deduct that from the total cost before working out the cost per delegate?

Deepak: That sounds right. Chris, would you know if the cost was right if we gave you the numbers and results we get from the program?

Chris: Sure.

Deepak: OK, let's do it.

Pete: I'll change the XML file by adding a sponsor payment node.

Pete edits the XML file to look like this.

<Courses> <London> <Venue>3000</Venue> <Catering>30</Catering> <Equipment>2500</Equipment> <Speaker>2000></Speaker> <MaxAttendees>30</MaxAttendees> <SponsorPayment>900</SponsorPayment> </London> </Courses>

Deepak: Hold on. What are you doing? I think we should change the XML file format. It is getting very messy with all these special cases.

Pete: Why? I thought we should just do the simplest thing. That's what Eddie keeps telling us. It's easy if we just add another if statement to the bit where we get all the data for a location.

Deepak: I have always interpreted "do the simplest thing" as meaning in the code. So it's the code that should be simple, not us!

Pete: You know you can be quite funny at times, and that wasn't one of them.

Deepak laughs, partly from embarrassment and partly because he knows Pete is right. Deepak takes his job as seriously as Pete takes his fooling around.

Deepak: OK, let's do it your way and then we can refactor the code to make it simpler.

Pete: Alright.

Pete grins and cracks his knuckles before tapping out the following changes to the ExtractCostItems method.

private decimal sponsorPayment; private void ExtractCostItems(XmlNodeList nodes) { XmlNodeList children = nodes[0].ChildNodes; foreach(XmlNode item in children) { if (item.Name == "MaxAttendees") { attendees = Convert.ToUInt32(item.InnerXml); } else if (item.Name == "SponsorPayment") { sponsorPayment = Convert.ToUInt32(item.InnerXml); } else { CostItems Item = new CostItems(item.Name, Convert.ToUInt32(item.InnerXml)); costItems.Add(Item); } } }

Deepak: You see what I mean about getting messy?

Pete: (grinning) It looks like some of the best code I've ever written!

Deepak: At times I can believe you are telling me the truth, this isn't one of them!

Pete: Touche!

Pete carries on at the keyboard and makes the following changes to the TotalCost method.



private decimal TotalCost(decimal totalCost) { foreach(CostItems item in costItems) { if (item.Name == "Catering") { totalCost += item.Cost*attendees; } else totalCost += item.Cost; } totalCost -= sponsorPayment; return totalCost; }

Deepak: OK, we'll need to change the test as well now. Run the test and make sure it's broken.

Pete: Hey that's the wrong way around isn't it? Aren't we supposed to break the test and then fix the code?

Deepak: Well it was your happy fingers on the keyboard plowing into the code first! Let me drive for a bit.

Pete hands Deepak his keyboard.

Pete: OK, go ahead.

Deepak runs the test in NUnit GUI and it passes.

Pete: Hey great! See I told you it was easy.

Deepak: But that doesn't make any sense. There is no way the test should pass. What's going on here?

Pete: Hold on! We forgot to copy the XML file to the root of the C: drive.

Deepak: Man this is a mess.

Deepak copies the test.xml file to the C: drive and runs the test again. This time the test fails.

Pete: So how do we make the test pass now?

Deepak: We're doing this the wrong way around but now we have to change the test to reflect the changes we just made to the code.

Deepak changes the expected result in the test so it now looks like this.

[Test] public void GetCourseCost() { CourseCost cCost = new CourseCost(@"C:\Test.xml"); decimal cost = cCost.GetCost("London"); Assert.AreEqual(500, cost, "Cost is not correct"); }

They run the test again, this time it passes.

Deepak: OK, we're back on track now.

Pete: Hold on, I've just thought of something.

Deepak: What?

Pete: Chris? Can a course have more than one sponsor?

Chris: Yes and yes!

Deepak: OK!

Pete: Now we really need some help.

Deepak: I knew this mess was going to cause trouble. We should get that XML file into shape first. Then we can get the code to work.

Pete: Huh? That doesn't sound right.

Deepak:Well do you have any better suggestions?

Pete: Um… nope.

Deepak: Right, so lets do this and then we can always change it later.

Deepak edits the XML file so that it contains a hierarchy of the costs as shown.

<Courses> <London> <MaxAttendees>30</MaxAttendees> <FixedCosts> <Venue>3000</Venue> <Equipment>2500</Equipment> <Speaker>2000</Speaker> </FixedCosts> <VariableCosts> <Catering>30</Catering> </VariableCosts> <Sponsors> <CoDe>900</CoDe> </Sponsors> </London> </Courses>

Pete: That looks good.

Deepak: Well it's more descriptive of what the costs actually are.

Pete: I get it. We'll work with the categories of costs and then use the total from each category to calculate the total cost!

Deepak: Right. Actually, categories is a good name to use.

In the ExtractCostItems method Deepak renames the XmlNodeList variable from children to categories.

Pete: Great, you should also rename the XmlNode item variable in the foreach loop. Call that "category."

Deepak: Well spotted.

Pete: You should also change the costItems array to be called fixedCostItems, capital C, capital I.

Deepak: Good thinking. I guess we should also change the sponsorPayments to an ArrayList.

Pete: Yeah, and add a new ArrayList called variableCostItems.

Deepak: We should initialize them in the constructor like this.

public CourseCost(string Filename) { srcFile = Filename; fixedCostItems = new ArrayList(); variableCostItems = new ArrayList() ; sponsorPayments = new ArrayList(); }

Pete: Sweet! Hey, can I drive for a bit? I think I know what we need to do in the ExtractCostItems method.

Adding the Finishing Touches
Deepak hands Pete the keyboard.

Deepak: Sure, go ahead.

Pete changes the if-else conditions in the foreach loop to a switch statement as shown in Listing 3.

Deepak: Oh no it's getting worse, not better!

Pete: Relax man. This is exactly what I planned. Watch this. . .

Pete uses the Extract Method on the code inside the fixed costs case of the switch statement. He calls the new method GetCategoryCosts. It looks like this.

private void GetCategoryCosts(XmlNode category) { foreach(XmlNode cost in category.ChildNodes ) { CostItems Item = new CostItems(cost.Name, Convert.ToUInt32(cost.InnerXml)); fixedCostItems.Add(Item); } }

Deepak: OK, I think I see what you are doing.

Pete: Good, but this Resharper tool wouldn't let me pass the ArrayList into the extracted method. Stupid thing, now I have to actually write some more code!

Pete changes the method to use an ArrayList passed as a parameter rather than as the fixedCostItems collection.

private void GetCategoryCosts(XmlNode category, ArrayList categoryList) { foreach(XmlNode cost in category.ChildNodes ) { CostItems Item = new CostItems(cost.Name, Convert.ToUInt32(cost.InnerXml)); categoryList.Add(Item); } }

He then changes the call in the FixedCosts case to pass the fixedCostItems variable into the newly extracted method.

case "FixedCosts": GetCategoryCosts(category, fixedCostItems); break;

Deepak: Hey pretty smart, now we can do the same with the other case statements!

Pete: Yes indeed.

Pete edits the rest of the cases in the ExtractCostItems method so that it now looks like this:.

private void ExtractCostItems(XmlNodeList nodes) { XmlNodeList categories = nodes[0].ChildNodes; foreach(XmlNode category in categories) { switch (category.Name) { case "MaxAttendees": attendees = Convert.ToUInt32(category.InnerXml); break ; case "FixedCosts": GetCategoryCosts(category, fixedCostItems); break; case "VariableCosts": GetCategoryCosts(category, variableCostItems); break ; case "Sponsors": GetCategoryCosts(category, sponsorPayments); break; default: break; } } }



Deepak: Super cool! That is looking much better. We'll need to change the TotalCost method now to use the values in the collections.

Pete edits the TotalCost method to use the collections as shown below:

private decimal TotalCost(decimal totalCost) { foreach(CostItems item in fixedCostItems) { totalCost += item.Cost; } foreach(CostItems item in variableCostItems) { totalCost += item.Cost*attendees; } foreach(CostItems item in sponsorPayments) { totalCost -= item.Cost; } return totalCost; }



Deepak: OK, does the test still run?

Pete runs the test in NUnit and it passes.

Deepak: Looking good!

Pete: So are we done?

Deepak: I think so. We've added the functionality that Chris asked for.

Pete: But I'm getting the hang of this now. I reckon there are more places we could tidy up the code here.

Deepak: We probably could but the code is doing what we want now. I think we should leave it until we need to add another new feature to this class library.

End of Day Wrap Up
At the end of the day the team gathers to discuss how they are doing using Resharper.

Eddie: So do you guys reckon Resharper helped at all?

Pete: I'm not sure. It seems to have a lot of other cool features. It brings up warnings as you're typing the code.

Deepak: I think it would be more useful in a big project when you want to zip around the code and find out where methods are called from.

Pete: The refactoring tools are really only a small subset of the Resharper toolkit and if we only buy it for that, then it might not be the best use of our money.

Eddie: So as we extend this project you think it will become more valuable?

Deepak: Yes I think that as we start to create more dependencies between classes we'll find more use out of the toolkit.

Chris: OK, it sounds like we ought to investigate if we can get licenses for you guys.

Deepak: Well, we can download and use the trial for a month and then make a decision.

Chris: Why don't you do that? It will probably take that long to raise a purchase order anyway.

Eddie: Great work team! Who's up for pizza and a beer?

Sue: (sighing) You boys!



Dr. Neil Roodyn is an independent itinerant consultant, trainer, and author. He travels the world working with software companies. Dr. Neil loves Australia, where he spends the summer enjoying the Sydney lifestyle and helping software development teams get more productive. Dr. Neil spends his other summer each year flying between northern Europe and the USA, working with software teams and writing about his experiences. Neil brings his business and technical skills to the companies he works with to ensure that he has happy customers. You can find out more from his Web site.
Comment and Contribute

 

 

 

 

 


(Maximum characters: 1200). You have 1200 characters left.

 

 

Sitemap