I have inherited and now further develop a large application consisting of an ASP.NET application, VB6 and VB.NET application.
The software was poorly written. I am trying to refactor the code as I go along. The changes I am making are not live (they are contained in a folder on my development machine). This is proving to be time consuming and I am doing this along side other work which is the prioritiy.
My question is: is this a practical approach or is there a better methodology for refactoring code? I don’t have any experience with version control software or source control software and I am wandering if this is what I am missing. I am a sole developer.
3
Refactoring or not, you should be using source control. It will save your bacon whether the team is one developer or one thousand.
If you try to maintain two concurrent branches of development the current version and the mythically beautiful refactored version, you will never get the codebase refactored the way you want it. It makes far more sense to refactor code has you have a reason to touch it. Carve a little extra time here and there for it. When someone asks you to fix a bug in module A, you refactor a few things. Here and there. Little by little.
3
Before doing any other refactoring I would start with the basic (which many seems to forget)
Fix the exception handling
I’ve seen loads of applications that “handles” exceptions. i.e. they try the exception, log it and then rethrow it (and if you are lucky without doing throw ex;
). An exception is really handled when the method can deliver the result that it promised. Those are the only catch blocks that should be left.
Remove all those try blocks. In ASP.NET you can catch (and log the exception + the request) in global.asax.
Getting proper exceptions is vital when refactoring (so that you know where you screw up).
Validate arguments
This is another common problem. Many developers seem to think that they got full control of their application and what they pass to each method. Hence they do not validate the method arguments.
Be polite, throw early. Doing so will also reduce the need for basic logging since the error will be caught early in the call stack instead of deep down.
A bonus is that you have to spend time understanding the methods to be able to validate the arguments. You might even want to document them with your findings until next time you visit them.
Continue on
When that’s done you might continue with the rest like refactoring large methods etc.
7
Refactoring should be done on your live project — as part of other work. This has several advantages, most importantly, it actually gets done. Almost as important, your changes are rolled in slowly, so that when you miss something, and break it (and you will), there’s one thing brocken and not working and not a hundred.
On a related note, it allocates the “cost” of the refactoring appopriately –if you have a module that is either hardly used, or ugly but problem free, fixing it is less important than a module that is used a lot or actually impeding new development. So when you are fixing bug X, or adding feature Y, and have to work around prblems in class z, z should be cleaned up.
I think you should learn source control so that if you make a change that breaks another component, you can easily roll back to the previous version.
With respect to how you are refactoring, there really isn’t a better methodology. It is a time consuming process but, when done right, is well worth it. In the book Clean Code, i feel the author has a great approach to refactoring. He suggests that everytime you need to make a change in a file, to also refactor and leave the file in better shape than you found it. This approach can allow you to work on your task then since you are familar with code, it will be easier to refactor.
I am in a slightly similar situation(code is probably much better quality, but we just now dropped support for .Net 1.0)
What we do that works well for us is refactor as we go and spend any free time refactoring. So… Say, you have a function which puts the title on the page or some such. Now, your boss needs you to alter that title function so that the website’s name is included in it, not just the specific page. So, you go to fix it and find that there are 5 different Title functions which do similar things. You should refactor those to 1 function so you don’t have to fix it in 5 places. You might also find that a PageTitle class might need to be made instead or something
I’m sure some people will disagree with me, but sometimes, working code is good enough. If you have a huge 1000 line method that you haven’t had to change in 5 years, and it doesn’t cause big integration problems, why change it? Most likely it won’t be touched for the next 5 years after you refactor it as well. You really need to have priorities when refactoring on a schedule. This is the general order I follow
- Code that I’m currently modifying (usually scoped to a class or two)
- Code that I don’t necessarily have to modify, but that causes integration pains in code I’m having to write.
- Code that is difficult to read
- Code that is not easy to read, but never breaks.
- (ie, never usually) code that could be cleaner, but otherwise is decently readable and modifiable.
Real life example of these priorities. There is a parser in the product I maintain that is generated from a grammar file using some tool. It’s the most horrific code I’ve ever seen, but it hasn’t been touched in 5 years or more. I don’t expect to ever have to touch it either because it’s never broke. (and we have tests to make sure of that).
However, we have a very big pain point in our .Net 1.0 to modern conversion. Some genius decided to use IEnumerator
instead of IEnumerable
throughout the code base. This prevents foreach loops from working. Almost every chunk of code I write I end up coming across needing to use one of these IEnumerator’s. Instead of continuing to use this anti-pattern, I fix it. Always. Sometimes, it requires modifying 50 or more lines of code, but it’s something that lowers technical debt where it matters.
And I’ll leave one last point. USE VERSION CONTROL. Trust me! If you’re the only maintainer and management doesn’t care about it, then use git or mercurial. You don’t even need a central repository for these and still get most of the benefits. Version control is invaluable when you decide to embark on refactoring something and find out it significantly breaks things. With version control, you roll back the last commit. Without, you cross your fingers you made a copy of the directory recently.
Actually you are on a right path by mentioning the lack of source control in your development practice/company. This is a must to have for a SOLID application development and proper project maintenance work.
How would you know what set of changes brake the legacy logic? and how would you roll back to previous state without using a version control tools like Git, TFS, SVN, etc ?…
Also better NOT to maintain two different code basis of a project. It is error prone and pain in merger of the code, especially a bigger pain in a team development.
For re-factoring: Install a tool for code refactoring (like, JustCode, Re-sharper, etc..). There are some free add-ons that you may get from NuGet packages as well. It will save you ton of times and provide you hints to take care of the smelling code blocks.