Re^2: How to deal with old OO code that mixes instance methods with class methods

Is it working?

Well, it is in production. :) I have been tasked with fixing a number of reported bugs. It qualifies as legacy code in that:

It does not use strict or warnings.
No unit tests.
Hard to test in isolation. Not designed with testability in mind.
No documentation.
No coherent error handling policy.
Lots of code duplication.
Big-arse functions.
Functions that are not cohesive (similar to a sin_and_tan() function).
Inconsistent naming. Poorly chosen names.
Lots of unnecessary cleverness.
Code that does not match the comments.
Pointless code (e.g. checks if running on Unix at runtime yet contains "use Win32::..." and so won't even compile on Unix).
Constructor does not check validity of arguments.
Module does not contain a version.
Oh, and perl critic found a number of flaws such as: i) open calls not being checked; ii) $_ being modified in map and grep blocks; iii) capture variables being used without checking if regex succeeded.

Comment on Re^2: How to deal with old OO code that mixes instance methods with class methods Select or Download Code

Replies are listed 'Best First'.
Re^3: How to deal with old OO code that mixes instance methods with class methods by AnomalousMonk (Archbishop) on Dec 21, 2013 at 19:03 UTC
[The module has] a number of reported bugs. Ok, so it's broke. [The module has] no unit tests. This is the first problem I would address: Nobody touch nothin' until you figure out a way to test what you 'fix'. Only after you can test any changes should you address the reported bugs. After resolving any code changes necessitated by these bugs, I would consider relapsing into "no broke, no fix" mode. Of course, if you're a programmer worth your salt, the list of 'problems' you have presented will greatly tempt your intervention. The issues most likely to draw my attention would be those bearing on maintainability. You must recognize, however, that you are venturing upon a slippery slope and may, willy-nilly, find yourself in full-on "can't fix just one" mode. Caveat programmor.	[reply]
Re^4: How to deal with old OO code that mixes instance methods with class methods by davido (Cardinal) on Dec 21, 2013 at 19:14 UTC
This! Oh, and Skimmable Code (pdf) It's a big PITA, but figure out what one giant function takes as inputs (included those absorbed through global osmosis), and what it excretes through returns and global manipulations. Figure out its side effects (opening of filehandles, etc). Then write regression tests for the function. Next, follow Schwern's skimmable code techniques to break the big functions into smaller testable units, while writing unit tests to verify the functionality of these smaller units. Throughout the process, keep running the original regression tests. Finally you'll be left with something that you can look at and understand how to fix. Most likely the fixes won't involve removing `my $self = shift;` -- that's just a red herring. The elephant in the corner is the humongous functions that probably don't strictly adhere to simple parameter passing. Dave	[reply] [d/l]
Re^3: How to deal with old OO code that mixes instance methods with class methods by BrowserUk (Patriarch) on Dec 21, 2013 at 19:52 UTC
What would be really interesting (to me at least; but I think widely), would be to post the original code with sufficient context/data to allow it to be run, and solicit as many people as possible to post their reworking of it. You could then pick'n'mix for your own solution; and the rest of us could compare our solutions with those of others. It would be a rare opportunity for everyone, regardless of their expertise, to learn from others. With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday' Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error. "Science is about questioning the status quo. Questioning authority". In the absence of evidence, opinion is indistinguishable from prejudice.	[reply]