views:

103

answers:

2

After months of frustration and of time spent in inserting needles in voodoo dolls of previous developers I decided that it is better try to refactor the legacy code.

I already ordered Micheal Feather's book, I am into Fowler's refactoring and I made some sample projects with DUnit.

So even if I don't master the subject I feel it is time to act and put some ideas into practice.

Almost 100% of the code I work on has the business logic trapped in the UI, moreover all is procedural programming (with some few exceptions). The application started as quick & dirty and continued as such.

Now writing tests for all the application is a meaningless task in my case, but I would like to try to unittest something that I need to refactor.

One of the complex tasks one big "TForm business logic class" does is to read DB data, make some computations and populate a scheduler component. I would like to remove the reading DB data and computation part and assign to a new class this task. Of course this is a way to improve the current design, it is not the best way for starting from scratch, but I'd like to do this because the data returned by this new class is useful also in other ways, for example now I've been ask to send e-mail notifications of scheduler data.

So to avoid a massive copy and paste operation I need the new class.

Now the scheduler is populated from a huge dataset (huge in size and in number of fields), probably a first refactoring step could be obtaining the dataset from the new class. But then in the future I'd better use a new class (like TSchedulerData or some other name less bound to scheduler) to manage the data, and instead of having a dataset as result i can have a TSchedulerData object.

Since refactor occurs at at small steps and tests are needed to refactor better I am a little confused on how to proceed.

The following points are not clear to me:

1) how to test a complex dataset? Should I run the working application, save one result set to xml, and write a test where I use a TClientDataSet containing that xml data?

2) How much do I have to care about TSchedulerData? I mean I am not 100% sure I will use TSchedulerData, may be I will stick with the Dataset, anyway thinking of creating complex tests that will be discarded in 2 weeks is not appealing for a DUnitNewbee. Anyway probably this is how it works. I can't imagine the number of bugs that I would face without a test.

Final note: I know someone thinks rewriting from scratch is a better option, but this is not an option. "The application is huge and it is sold today and new features are required today not to get out of business". This is what I have been told, anyway refactoring can save my life and extend the application life.

+2  A: 

I'd say approach it in focussed baby steps.

Step#1: Should always be to get some tests around your area of invasion TForm - regression tests aka safety net. In your case, sense what the app is doing. From what I read, it seems to be a data transformer. So spend time to understand all (or most important if all is not feasible) combinations of input data and the corresponding output schedules. Write them up as tests. Ensure that all tests pass.

Step#2: Now attempt your refactorings. Move blocks of code into cohesive classes etc all under the safety of the regression net.

Testing complex datasets - testing file dumps should be the last resort. But in this case, it seems like a simple option to get started. Maybe you could later make it a first class domain object TSchedule with its own Equals() implementation. Defer design decisions/changes until you have a solid regression test suite around your area of modification.

Gishu
Ok thanks. The fear I have is that writing tests will take lot of time, because the data is complex. File dumps are complex, so being this my first real life unittesting attempt I fear to lose a week and ending up with unsuccesful tests. Anyway I have no other place to start, I mean, I need to change THIS CODE and so I should start from here.
+1  A: 

Your eventual goal is to separate the UI, data storage and business logic into distinct layers.

Its very difficult to test a UI with automatic testing frameworks. You'll want to eventually separate as much of the business logic from the UI as possible. This can be accomplished using one of the various Model/View/* patterns. I prefer MVP passive view, which attempts to make the UI nothing more than an interface. If you're using a Dataset MVP Supervising Controller may be a better fit.

Data storage needs to have its own suite of tests but these are different from unit tests (though you can use the same unit testing framework) and there are usually fewer of them. You can get away with this because most of the heavy lifting is being done by third party data components and a dbms (in your case T*Dataset). These are integration tests. Basically making sure your code plays nice with the vendor's code. Also needed if you have any stored procedures defined in the DB. They are much slower that unit tests and don't need to be run as often.

The business logic is what you want to test the most. Every calculation, loop or branch should have at least one test(more is preferable). In legacy code this logic often touches the UI and db directly and does multiple things in a single function. Here Extract Method is your friend. Good places to extract methods are:

for I:=0 to List.Count - 1 do
begin
  //HERE
end;

if /*HERE if its a complex condition*/ then
begin
  //HERE
end
else
begin
  //HERE
end

Answer := Var1 / Var2 + Var1 * Var3; //HERE

When you come across one of these extraction points

  1. Decide what you want the method signature to look like for your new method: Method name, parameters, return value.
  2. Write a test that calls it and checks the expected outcome.
  3. Extract the method.

If all goes well you will have a newly extracted method with at least one passing unit test.

Delphi's built in Extract Method doesn't give you any way to adjust the signature so if that's your own option you'll have to make do and fix it after extraction. You'll also want to make the new method public so your test can access it. Some people balk at making a private utility method public but at this early stage you have little choice. Once you've made sufficient progress you'll start to see that some utility methods you've extracted belong in their own class (in which case they'd have to be public anyway) while others can be made private/protected and tested indirectly by testing methods that depend on them.

As your test suite grows you'll want to run them after each change to ensure your latest change hasn't broken something elsewhere.

This topic is much to large to cover completely in an answer. You'll find the vast majority of your questions are covered when that book arrives.

codeelegance