30

I was assigned to a new project recently. Well, an old project actually, written in classic ASP. Now a new version of the application is being written in the latest ASP.NET, but it's not expected to be RTM in a while (estimated release date is January 2017) so I have to perform some maintenance on the old application until it can be discarded.
Also, I've got a feeling that not all customers will be switching over to the new program immediately, so this version will probably be around for a while.

And the problem is, it's full of errors. Parts of it date back to the previous century, when there were no web standards, and I don't really mind about Quirks mode, and width and height attributes instead of CSS, tables used for layout, framesets etc, but oh, all those errors! width="20px" all over the place, onchange="javascript:...", and in those places where they do use css, style="width:20" and style="width=20px" are commonplace. Not to mention plenty of lines where there are contradictory width and style attributes. Etc etc.
As a result, the web application only runs under IE, and only in compatibility mode. It's clear that the developers never looked at code validity, only if what came out looked like what they had in mind it should look like.

And I don't know how to handle that. I find it impossible to close my eyes to those errors while looking in the code for other errors.
I can of course do a global find and replace to get most of the issues out of the way, but that would mean my first commit would consist of thousands of changed .asp files. Can I do that?

Mr Lister
  • 1,599
  • 3
  • 12
  • 18
  • 2
    Is a rewrite possible? Classic ASP is very difficult to support nowadays... It wouldn't surprise me if Microsoft doesn't even recognize it as a supportable paradigm anymore. If you can fix this with a global find-and-replace, do it and count your lucky stars. – Robert Harvey Jun 19 '16 at 14:23
  • 1
    A rewrite (from scratch) is possible; that's what they are doing right now, but in the meantime the old application must be maintained, which is where I came in. There are about 1400 asp files in total, as well as several hundred other supporting files (html and css) with problems. – Mr Lister Jun 19 '16 at 14:26
  • If you're worried about the find and replace causing the site to possibly fall down, run a copy of it side by side until you get the changes right. – Robert Harvey Jun 19 '16 at 14:28
  • 21
    by "errors" do you mean coding style you dont like? – Ewan Jun 19 '16 at 14:28
  • @Ewan No, I meant the actual errors. I can live with quirks mode, tables used for layout, bordercolor attributes and so on, honestly. OK, maybe I shouldn't have mentioned the HTML 3.0 doctype declaration... – Mr Lister Jun 19 '16 at 14:31
  • @MrLister: Are you sure it it not a HTML 3.2 doctype? There is no HTML 3.0 standard (for historical reasons), but there is a perfectly valid HTML 3.2 standard. – JacquesB Jun 19 '16 at 16:04
  • 9
    A tip that I heard: Go to a place where music students practice. Try to get fifteen minutes in a soundproof room. SCREAM for fifteen minutes. Now you feel better, go and fix the bugs! Seriously, check with management what the goal is. If this software is needed, it may very soon stop them from upgrading computers and cause problems replacing older broken computers. – gnasher729 Jun 19 '16 at 17:46
  • 24
    This question sounds more like a rant. Why are you complaining about a software which will be disposed in a few months? – Doc Brown Jun 19 '16 at 19:18
  • 5
    It is not an "error" that code written in "classic ASP" follows the standards (such as they were) of classic ASP, which happen to be different from the latest fashion in web coding - and the latest fashion will probably be "out of date" by next year in any case. "It's clear that the developers never looked at code validity" - if the OP thinks he/she can write code that will still "look valid" 15 or more years in the future, time will tell if that belief is just the natural optimism (or ignorance) of youth. – alephzero Jun 19 '16 at 20:26
  • 4
    @alephzero Again, I can live with following the standards of 15 years ago, the attributes that would be CSS properties these days, the framesets. What I can NOT live with is that they didn't follow any standards! It looks like they just played around a bit until it appeared to work, leaving their successors to solve this mess! – Mr Lister Jun 19 '16 at 20:52
  • 3
    @DocBrown Alright, so it IS a rant. Do you think this question should be deleted? Anyway, my gu feeling says this old version will be around for a while; I'm not convinced we can get all our customers to switch immediately after introducing the new one. – Mr Lister Jun 19 '16 at 20:55
  • ...okay, I feel bad for you, especially for having to use ASP in general, but what's your question?? – cat Jun 19 '16 at 21:31
  • 1
    @MrLister: deleted? No, but improved by removing the ranting part, and by adding the information to the question how long you expect to need for maintenance of the current version. Your current questions pretends in about six to eight weeks is the end of the maintenance period. – Doc Brown Jun 19 '16 at 22:02
  • 1
    @Ewan Since he describes things that were never, at any point, valid in HTML, I don't think "coding style he doesn't like" is really the issue. – Casey Jun 20 '16 at 01:55
  • @DocBrown I edited, but I find I can't remove the rant without changing it into "how do I debug a classic ASP application" which isn't really the problem. – Mr Lister Jun 20 '16 at 06:21
  • 19
    "I have to perform some maintenance on the old application until it can be discarded." What maintenance? Please be specific. If you were tasked with maintaining this code base, and nothing else was said, then do not change a single thing. Maintaining it implies that you keep on making it work, not correct things that are not considered broken in the first place. – Stephan Branczyk Jun 20 '16 at 06:33
  • 4
    @StephanBranczyk - This is exactly right. It is a _business decision_ how much and what kind of effort to put into keeping an _existing production system_ running. OP needs to talk with his managers and get their precise criteria. I'm currently working maintenance on a production system which is responsible for well over $1B/year revenue - while the system that will replace it is being written. Lost revenue >> $100000 per hour of downtime. The overriding goal is _no downtime_ therefore the _only_ rule is _touch the minimum necessary to fix a reported bug_. Swallow it and smile on payday. – davidbak Jun 20 '16 at 17:37
  • May the force be with you – Panos Kalatzantonakis Jun 20 '16 at 17:49
  • 2
    Possible duplicate of [I've inherited 200K lines of spaghetti code -- what now?](http://programmers.stackexchange.com/questions/155488/ive-inherited-200k-lines-of-spaghetti-code-what-now) –  Jun 20 '16 at 17:56
  • 3
    Don't touch anything you don't need to. You'd deservedly get fired. Why do you think they're replacing it? Everybody else that's ever touched this codebase, including the ones who wrote it, also thought of what you are thinking of. Sorry to sound rude but I hope you avoid making a terrible mistake. I'm not saying you're not smart; just everybody else is pretty smart too. And they know why this plan won't work. Note: for a potentially 20 year old app... having a replacement in 2017 is practically tomorrow. – FastAl Jun 20 '16 at 22:20

6 Answers6

100

It sounds like you are confusing several things into the term "errors"

  • legacy html attributes
  • coding style
  • coding errors which don't cause bugs
  • unreported bugs
  • mistakes which are now features
  • reported bugs
  • reported bugs you have been assigned to fix

On a legacy app which is going to be replaced only one of these types of error should concern you. The last one.

I would go as far as to say you shouldn't even refactor other stuff on a feature you are bug fixing, mainly due to :

  • mistakes which are now features

You can see from the code how it was maybe meant to work but never did, but all the users have been getting along with the indeterminately widthed element for the past 10 years and they wont thank you for fixing it.

On the plus side, if you put your cynical JFDI head on, you will be able to burn though the bugs super quick and the new version team wont be able to keep up with the old versions features.

This will give you a wry gloating smile of ironic glee as you recommend a chrome plugin ie6 emulator to clients so they can keep using the marquee 'feature' they love

Ewan
  • 70,664
  • 5
  • 76
  • 161
  • 28
    "*mistakes which are now features*" - oh, the joy... – F.P Jun 20 '16 at 08:04
  • 36
    Indeed be very cautious in what you touch, This immediately came to mind: https://xkcd.com/1172/ – Dennis Jaheruddin Jun 20 '16 at 09:38
  • 3
    Please clarify `... JFDI ...` – GER Jun 20 '16 at 13:08
  • 5
    @GER "Just [expletive] Do It" which means to eschew normal standards and testing and other things and just get a fix in without caring whether it's done in a maintainable and readable way. – Nzall Jun 20 '16 at 13:22
  • 3
    like aglie but more so – Ewan Jun 20 '16 at 13:33
  • @NateKerkhofs I was hoping that stood for a technical term, but that makes sense to just get the requests done. – GER Jun 20 '16 at 13:58
  • @GER In this context it actually is a viable tactic. The web app is being deprecated in a few months anyway, so any effort invested in trying to improve it beyond fixing bugs would be better spent on fixing more bugs. – Nzall Jun 20 '16 at 14:09
  • @Ewan I think it would be worthwhile to highlight "On a legacy app which is going to be replaced" in your answer. I agree with what you said if the system will soon be replaced. However, on a legacy app that will still be used for several years/decades, I think it is laudible to improve the overall codebase (if not to make your job easier, then to make the job for the next guy easier). – NoseKnowsAll Jun 20 '16 at 16:46
  • 3
    +1 - mostly. The only thing I disagree with is that you have a missing item in your list: unreported security errors. I have never seen an application of the type described that doesn't have at least 1 if not actually hundreds of security holes (the last time I worked on a site that looked like this, each and every one of hundreds of pages contained SQL injection bugs). These should be fixed on discovery without requiring specific clearance for each and every one - get blanket clearance for all of them. – Jules Jun 20 '16 at 20:54
  • security holes do complicate the question significantly. As you say, you are probably legaly required to fix these. But what if they are known about and signed off as acceptable? – Ewan Jun 21 '16 at 07:31
  • 2
    @FlorianPeschka I was amused when I saw that in one of our older projects the filepath was correct just because someone mistakingly used $_SREVER instead of $_SERVER – tsuma534 Jun 21 '16 at 07:33
39

What you ask is not a technical question, and nobody here can answer it.

You are working on a piece of software in maintenance mode, and you observe out-of-fashion technology and a large number of imperfections and inconsistencies. You ask what to do. Should you for example extend effort to make it cross browser compatible? Should you bring it in line with modern standards? Should you fix syntactic inconsistencies across the app? The thing is, these are business decisions. You should ask your manager or product owner what problems they want you to solve and what their priorities are. Since there already is a project underway to rewrite the app, management is most likely already aware of the problems you observe.

If the app will be full replaced in a matter of months, it is likely they only want you to fix specific critical problems and leave the rest of the mess alone. But we don't know.

You ask if you can make sweeping search-and-replace operation across the codebase, changing thousands of files. Of course you can. The question is if you should. Such sweeping changes will likely require extensive testing to ensure nothing broke. Again it is a business decision if the benefit outweighs the cost in time and risk.

JacquesB
  • 57,310
  • 21
  • 127
  • 176
  • 1
    It's a business decision but it's so obvious to answer that he does not need to ask the manager. He should not clean up the mess if not required. (+1) – usr Jun 20 '16 at 09:29
13

When the application will be replaced in 18 to 24 weeks (adding the to be expected delays to the 6 to 8 week estimated presented above) then you really need to ask yourself what value you add to the business by still investing considerable amount of work in the old version.

Sure, when you would be stuck with supporting the application for several years to come, then getting rid of the technical debt can be worth it in the long run. But when all of it is going to be dumped anyway, then why bother? Just add another hackish fix on top of all the other hackish fixes to repair whatever problem simply can't wait until the release of the new version and call it a day.

You might also ask yourself what you can do for the application in the little lifetime it still has left. When you are really bored right now and simply have nothing better to do with your time you could give it a grand overhaul and remove all the style problems you mentioned, but it is very likely that this will at first break more things than it will fix. You might be able to get rid of these new problems eventually given enough time, but you don't have that time.

Philipp
  • 23,166
  • 6
  • 61
  • 67
  • 11
    `s/weeks/years/` – CodesInChaos Jun 19 '16 at 18:06
  • 9
    Last year I was fixing a performance bug that essentially amounted to a table that should cache some recent values actually keeping all history and growing boundlessly. In the appropriate place in the code, there was a comment saying essentially "this should be cleared periodically, but doesn't matter as we plan to scrap the system by the end of 2007". Nothing lives longer than temporary solutions. – Peteris Jun 20 '16 at 08:33
  • @Peteris Well, temporary taxes. But yeah. – Jay Jun 20 '16 at 17:26
  • Last time I worked on an application like this one, it was also destined to be temporary. The piece of hardware it was designed to control was being scrapped and a replacement built, and new software was to be developed to control the replacement., which would be available in 6 months. Unfortunately, the replacement hardware was faulty, and all the budget was spent attempting to fix the faults, so there was none left for the replacement control system. After a few years, the entire project was scrapped. AFAIK, the entire system still runs on both old hardware and software, 5 years later. – Jules Jun 20 '16 at 20:59
  • Fortunately, I got clearance to fix the worst of the issues (the SQL injection attacks, the SQL tables with millions of rows but *no indices*, the pages where the original developer had forgotten to check for authorization...). – Jules Jun 20 '16 at 21:01
3

Reasons NOT to make big changes:

One: The code is going away in a few months. Would it really be worth the company's time for you to spend 5 months fixing a system that will then be thrown away 1 month later? Caveat: Systems rarely go away when they are schedule to go away. The replacement system is almost always late, there are users who cannot upgrade for whatever reason, etc. But this is a complex issue.

Two: If you make a lot of changes, especially mass search and replaces, you will introduce bugs. Not you might introduce bugs: you will. Suppose you did an S&R and changed "width=200" to "width:200px". Is there C# or VB code on your ASP pges? Because if you had a variable named "width" that you were setting to 200, you just broke it. (Or for that matter, did you think to limit the S&R to ASP pages?) Or if you changed "width:200" to "width:200px", what happens if there was one place in the code that said "width:200mm"? Now it says "width:200pxmm". Okay, let's suppose you thought of those. What if there's someplace that had the invalid width specification, which is of course ignored, and it is now laying out quite nicely. You "fix" the width and it now lays out with 200px ... and the display is screwed up, because 200px is in fact the wrong width to give and it only worked because that value was ignored? Mass S&R's are very dangerous, because you are almost certainly not studying every place you change. You likely aren't even sure what to test.

Three: Code that is "obviously" wrong may in fact be what the user wants. I've seen plenty of requirements specs that call for behavior that is obviously wrong and insane ... and then I go back to the users and ask what they REALLY want, and it turns out that they really want this insane behavior, because that's how their business works or government regulations require it or whatever.

Even if the behavior really is wrong, maybe users have come to expect it and they routinely work around it, and by fixing it, you will break their workarounds. Example: I work on a system where we have a place where you specfy from and through dates that a sale is available to the public. Both dates were really the midnight that began that day, so if you said "through July 30" that meant it ended with the end of the day July 29, i.e. one minute before 12:01 am July 30, not the end of July 30. At one point I fixed this, but I could only do that because there were less than half a dozen people with authority to use that screen, and I could simply tell them all that I'd fixed it. If there were hundreds of users, and they'd all figured out by now that you really had to give the day after the through date, then my "fixing" it would have broken it for all these people.

Jay
  • 2,657
  • 1
  • 14
  • 11
0

I can of course do a global find and replace to get most of the issues out of the way, but that would mean my first commit would consist of thousands of changed .asp files. Can I do that?

I don't see why not. A commit should be conceptually one thing, but I see no reason why a global find and replace from style="width=20" to style="width: 20px" would not count as "one thing", conceptually speaking. And if it would help you sleep better, save you being distracted as you fix other things, and not muck anything up, why not?

TRiG
  • 1,170
  • 1
  • 11
  • 21
  • 12
    Why not? Because a sweeping search-and-replace on a large legacy codebase requires extensive testing afterwards to ensure nothing broke. – JacquesB Jun 20 '16 at 11:30
-2

Your problem is setting priorities: Which of the issues are showstoppers (in production)? Which are ticking timebombs? And which can be left in a while longer (because it works and has done so for years now, even sort-of)?

What I would do in your situation is would make lists of problem classes that I would like to look at. E.g., replacing style="width=(\d+)" with style="width: \1px" (which can probably be rectified with a global find/replace using regexp - sorry if mine is not 100%) would be one class, and if there's just one occurrence, so be it. For each category list a priority (how urgent is it to do this) and an estimate of work (how long will it take to do this category).

Your maintenance tasks will also figure in this list. Now you are starting to apply some management, even if only to yourself, and you have a tool to use when you have time on your hands and nothing to do, or when you need to negotiate with your manager about work to do (or request time to be allocated to something that he might not be aware of). (This sort of proactivity might help you to get noticed for promotions, if done right.)

I guess you enjoy programming because you have a slight perfectionistic personality to some extent. BUT in a commercial environment, you need to start to realize that perfect is the enemy of good (and good brings in the money, perfect might not necessarily bring in noticeably more for a whole lot more work). First do what is needed, then do what is nice to have. Yes, this might go against your grain no end. Just grin and bear it, and perhaps get a hobby to exercise your perfectionism and keep you sane ;-)

frIT
  • 193
  • 2