tags:

views:

790

answers:

10

We have a situation at work where developers working on a legacy (core) system are being pressured into using GOTO statements when adding new features into existing code that is already infected with spaghetti code.

Now, I understand there may be arguments for using 'just one little GOTO' instead of spending the time on refactoring to a more maintainable solution. The issue is, this isolated 'just one little GOTO' isn't so isolated. At least once every week or so there is a new 'one little GOTO' to add. This codebase is already a horror to work with due to code dating back to or before 1984 being riddled with GOTOs that would make many Pastafarians believe it was inspired by the Flying Spaghetti Monster itself.

Unfortunately the language this is written in doesn't have any ready made refactoring tools, so it makes it harder to push the 'Refactor to increase productivity later' because short-term wins are the only wins paid attention to here...

Has anyone else experienced this issue whereby everybody agrees that we cannot be adding new GOTOs to jump 2000 lines to a random section, but continually have Anaylsts insist on doing it just this one time and having management approve it?

tldr;

How can one go about addressing the issue of developers being pressured (forced) to continually add GOTO statements (by add, I mean add to jump to random sections many lines away) because it 'gets that feature in quicker'?

I'm beginning to fear we may lose valuable developers to the raptors over this...

Credit: XKCD

Clarification:

Goto here

alsoThere: No, I'm talking about the kind of goto that jumps 1000 lines out of one subroutine into another one mid way into a while loop. Goto somewhereClose

there: I wasn't even talking about the kind of gotos you can reasonably read over and determine what a program was doing. Goto alsoThere

somewhereClose: This is the sort of code that makes meatballs midpoint: If first time here Goto nextpoint detail:(each one almost completely different) Goto pointlessReturn

here: In this question, I was not talking about the occasionally okay use of a goto. Goto there

tacoBell: and it has just gone back to the drawing board. Goto Jail

elsewhere: When it takes Analysts weeks to decypher what a program is doing each time it is touched, something is deeply wrong with your codebase. In fact, I'm actually up to my hell:if not up-to-date goto 4 rendition of the spec goto detail pointlessReturn: goto tacoBell

Jail: Actually, just a small update with a small victory. I spent 4 hours refactoring a portion of this particular program a single label at a time, saving each iteration in svn as I went. Each step (about 20 of them) was smallish, logical and easy enough to goto bypass nextpoint: spontaneously jump out of your meal and onto you screen through some weird sort of spaghetti-meatball magnetism. Goto elseWhere bypass: reasonably verify that it should not introduce any logic changes. Using this new more readable version, I've sat down with the analyst and completed almost all of this change now. Goto end

4: first *if first time here goto hell, no second if first time here goto hell, no third if first time here goto hell fourth now up-to-date goto hell

end:

+19  A: 

How many bugs have been introduced because of incorrectly written GOTOs? How much money did they cost the company? Turn the issue into something concrete, rather than "this feels bad". Once you can get it recognized as a problem by the people in charge, turn it into a policy like, "no new GOTOs for anything except simplifying the exit logic for a function", or "no new GOTOs for any functions that don't have 100% unit test coverage". Over time, tighten the policies.

twk
Ahhh, unit test coverage... Now there is another problem...
Dan McGrath
To get rid of problem, get rid of people who write gotos incorrectly. There's nothing wrong in goto when done correct.
Cheery
+8  A: 

GOTOs don't make good code spaghetti, there are a multitude of other factors involved. This linux mailing list discussion can help put a few things into perspective (comments from Linus Torvalds about the bigger picture of using gotos).

Trying to institute a "no goto policy" just for the sake of not having gotos will not achive anything in the long run, and will not make your code more maintainable. The changes will need to be more subtle and focus on increasing the overall quality of the code, think along the lines of using best practices for the platform/language, unit test coverage, static analysis etc.

Igor Zevaka
I would read the list but it is blocked at work (I probably previously read it at home anyways). Let me assure you this usage of GOTO we are plagued with makes ** excellent** spaghetti code. After all, why not jump out of a loop from one subroutine into the middle of an if block in another subroutine...
Dan McGrath
Fair enough, that is pretty evil.
Igor Zevaka
A lot of people have forgotten the context of Dijkstra's letter. The problem wasn't the GOTO statement. The problem was that languages made in the 1960s often lacked basic control flow like "while" statements and you *had* to use GOTOs to get anything done.
dan04
@dan04: +1 for someone who's actually *read* and, more importantly, **understood** Dijkstra's letter.@Igor: +1 for pointing out that a "no `goto` policy" is meaningless given that one of the first things people reach for when `goto` is removed is the so-called "`godo`" abomination: `do { ... break; ... break; ... break; } while (false)` in C, for example.
JUST MY correct OPINION
@Just MY: Agreed. Saying not to do one bad practices because others do another bad practices is nonsensical however. Make them write their code properly! If a goto is REALLY called for, then fine, do it. Don't use it as your hammer for all nails though.
Dan McGrath
Dijkstra was also interested in formal static analysis, which gets the screaming heebies when a goto shows up(or memory access, or exceptions, or... ).
Paul Nathan
I agree with this. Focus not on the theoretical aspects of using/not using Goto's and instead look at the aspect of easier code maintainability.
gnlogic
I build static analysis tools. GOTOs don't make static analyis particularly hard; its easy enought to build a real flow graph of the code including GOTOs that go to dumb places.. What makes static analysis hard are crazy scoping rules (C++), complex or hidden dataflows (COBOL), and pointers to functions with casts (C), or (the only GOTO problem) and indirect GOTO. GOTOs make people crazy because the reference code is far away and doesn't fit anywhere in a program structure model a programmer might have. They don't bother static analyzers much, because such analyzers don't have any opinions.
Ira Baxter
@Dan McG: What language is your codebase in? The C standard mandates that `goto` is restricted to labels in the enclosing function only.
dreamlax
Dan McGrath
Unfortunately, goto hate has become a cargo cult. You can easily write spaghetti code without gotos, and I could even argue that many modern languages encourage it.
kyoryu
+1  A: 

I recently had to work on some code that wasn't legacy per se, but the former developers' habits certainly were and thus GOTO's were everywhere. I don't like GOTO's; they make a hideous mess of things and make debugging a nightmare. Worse yet, replacing them with normal code is not always straightforward.

IF you can't unwind your GOTO's I certainly recommend that you no longer use them.

LoudNPossiblyRight
+2  A: 

The reality of development is that despite all the flowery words about doing it the right way, most clients are more interested in doing it the fast way. The concept of a code base rapidly moving towards the point of imploding and the resulting fallout on their business is something that they cannot comprehend because that would mean having to think beyond today.

What you have is just one example. How you stand on this will dictate how you do development in the future. I think you have 4 options:

  1. Accept the request and accept that you will always be doing this.

  2. Accept the request, and immediately start looking for a new job.

  3. Refuse to do and and be prepared to fight to fix the mess.

  4. Resign.

Which option you choose is going to depend on how much you value your job and your self esteem.

Derek Clarkson
As a side note, there are situations where a GOTO is the correct answer. But they are very few and far between and nothing like the examples you give.
Derek Clarkson
I just attempted option 5, send a link to this question to the Anaylst. It got a laugh, then 10 minutes later I seen another goto statement being written into a spec... sigh.
Dan McGrath
A: 

If a change to the program requires "just one little goto" I would say that the code was very maintainable.

This is a common problem when dealing with legacy code. Stick to the style the program was originally written in or "modernize" the code. For me the answer is usually to stick with the original style unless you have a really big change that would justify a complete re-write.

Also be aware that several "modern" language features like java's "throw" statement, or SQLs "ON ERROR" are really "GO TO"s in disguise.

James Anderson
You've got it backwards. GOTO is a "throw" in disguise. Or a "break" in disguise. In general, a substitute for useful control structures that a language doesn't have.In older languages like pre-1980's versions of BASIC, "goto" was also used to implement "switch" in disguise, "while" and "do...while" in disguise, or even just multi-line "if" in disguise.That's where "GOTO Considered Harmful" came from. GOTO was used for so many different purposes it was hard to see what any particular GOTO was for.
dan04
If by maintainable you mean it takes the Analysts weeks to determine how the code works each time and it STILL has behaviors they don't have a clue on, then yes, it is very maintainable... Odd definition of maintainable though...
Dan McGrath
The OP did not say it took weeks for the analyst to work out the programs behaviours! If he did there may have been some justification for a rewrite. From what I gather the code just works.On a deeper level if the code contains GOTOs you cannot "properly" structure just one fragment, you have to get rid of all the GOTOs so a few more gotos dont really make any difference.
James Anderson
Most programs won't be diabolic to fix from the word go. Do a few iterations over a "few more GOTOs dont really make any difference" and pretty soon you have 4000 lines of pure evil genius. Giving in and saying "eh screw it" as sure as day isn't going to ever make it better. This just leads down the path where eventually you cannot do any changes in a reasonable time. Your just dealing the inevitable until a time it is even harder and more risky.
Dan McGrath
-2 huh, must remember never to offer an alternative opinion here!
James Anderson
As a side note to James' "The Op did not say": If you look, I think you'll find that since the comment you refer to was written by the OP (me), he did. :)
Dan McGrath
@dan04 - I like that: GOTO is a "break" in disguise. As in, it breaks your code. :-)
Carl Manaster
+2  A: 

Back to principles:

  • Is it readable?
  • Does it work?
  • Is it maintainable?
Carlos
No, Yes, Barely. 1/3. Is it readable? No cannot determine the business logic of the 80%+ of the code-base without in-depth analysis before commencing changes. This indicates pretty big issues right there... Does it work? Most of the time. Is it maintainable. In the same way all Spaghetti code is, you get there eventually through perseverance. There is a reason we have 19 developers (40+ in IT) for a 400 person company that ISN'T a software house or even IT related.
Dan McGrath
Sounds like it isn't just GOTO that's your problem. Your company needs to have a rethink about the codebase. Probably a re-design, re-write. They might also want to think about how much money they could save if they did things properly.
Carlos
Completely agreed, but rewriting 3000+ programs is out of the question, we have to attack them as well go. As it stands though, we can't even get a small win of making the ones we work on better, but instead are forced to make them worse! Just trying to win some small battles here first...
Dan McGrath
+2  A: 

This is the classic "management" vs. "techies" conflict.

In spite of being on the "techie" team, over the years I have settled firmly the "management" side of this debate.

There are a number of reasons for this:

  1. It's quite possible to have well written easy to read properly structured programs with gotos! Ask any assembler programmer; conditional branch and a primitive do loop are all they have to work with. Just because the "style" is not current and doesn't mean its not well written.

  2. If it is a mess then its going to be a real pain to extract the busines rules you will need if you are going for a complete re-write, or, if you are doing a technical refactoring of the program you will never be sure the behaviour of the refactored program is "correct" i.e. it does exactly what the old program does.

  3. Return on investment -- sticking to the original programming style and making minimal changes will take minimum effort and quickly statisfy the customers request. Spending a lot of time and effort refactoring will be more expensive and take longer.

  4. Risk -- rewrites and refactoring are hard to get right, extensive testing of the refactored code is required and things that look like "bugs" may have been "features" as far as the customer is concerned. A particular problem with "improving" legacy code is that business users may have well established work arounds that depend on a bug being there, or, exploit the existense of a bugs to change the business procedures without informing the IT department.

So all in all management is faced with a decision -- "add one little goto" test the change and get it into production in double quick time with little risk -- or -- go in for a major programming effort and have the business scream at them when a new set of bugs crops up.

And if you do decide to refactor in five years time some snotty nosed college graduate will be moaning that your refactored program is not buzzword compliant any more and demanding he be allowed to spend weeks rewriting it.

If it ain't broke dont fix it!

PS: Even Joel thinks this is a bad idea: things you should never do

Update!-

OK if you want to refactor and improve the code you need to go about it properly.

The basic problem is you are saying to the client is "I want to spend n weeks working on this program and, if everything goes well, it will do exactly what it does now." -- this is a hard sell to say the least.

You need to gather long term data on the number of crashes and outages, the time spent analysing and programming seemingly small changes, the number of change requests that were not done because they were too hard, business opertunities lost because the system could not change fast enough. Also gather some acedemic data on the costs of maintaing well structured programs vs. letting it sink.

Unless you have a watertight case to present to the beancounters you will not get the budget. You really have to sell this to the business management, not, your immediate bosses.

James Anderson
With all due respect, Joel is not always right. He even admits he doesn't agree with all of this old articles any-more. Also of note is that I'm not advocated scrapping all your code and going all Chuck Norris to round-house kick new code into existence. All I'm talking about is making an effort to ensure the code is in slightly better shape than before you touched it. Make it worse is just going to make the next change even harder... Then the next one harder, etc, until you get to have code like us.
Dan McGrath
Also, control structures other then *if( x ) goto* were introduced for a reason. Geez, by your logic, why don't we do all multiplication by looping and doing additions. It was how they use to do it... I've never-ever-ever argued that you cannot have well structure programs with gotos. I will argue though that you can problem have better structured and more readable and maintainable programs if you used meaning control structures.
Dan McGrath
Sorry to come on so strong with "live with it" argument but I have seen many people get badly burned trying to "improve" working code, and, most of the posts here were just not taking into account the difficulty involved.However it does sound like it is worth considering in your case, see the update to my post for some more constructive advice.
James Anderson
Joel's article supports pretty much the exact opposite of the point you are making. Just read the parts where he talks about refactoring: 'one programmer working carefully', '5 minutes with an emacs macro'He is saying no functioning code is so bad it can't be incrementally improved to be adequate from a business point of view. The conclusion is that you need to do the necessary maintenance to keep your existing software functioning. Dreaming of a clean-room rewrite is always a mistake.
soru
Management side is important, but when your most talented programmers get fed up and leave, you end up with a dead sea effect. Put a price on that!
Hamish Grubijan
+2  A: 

Maybe you can use the boyscout-pattern: Leave the place a little more clean than you found it. So for every featurerequest: don't introduce new gotos, but remove one.

This won't spend too much time for improvements, would give more time, to find newly introduced bugs. Maybe it wouldn't cost much additional time, if you remove a goto from the part, which although had to spend time understanding, and bringing the new feature in.

Argue, that a refactoring of 2 hours will save 20 times 15 minutes in the future, and allow faster and deeper improvements.

user unknown
That had been what we had been attempting to do. Our team is the large projects team, however, where large functionality "enhancements" are handled. This means most code we work with is either swaths of completed new or swaths of needing to be editing legacy code. In the later case, we were being pressured add new functionality to already bad code with equally bad goto 'hacks'. This meant when we needed to change it again in 6 months it would be even worse...
Dan McGrath
+1  A: 

Unfortunately the language this is written in doesn't have any ready made refactoring tools

Don't you have editors with macro capabilities? Don't you have shell scripts? I've done tons of refactoring over the years, very little of it with refactoring browsers.

Ken
Well, that brings back even more issues. In short, no. Yes, I have scripts I have written that actually clean up a lot of the mess of old programs, but in management's infinite wisdom, we are banned from using any tools we have written ourselves that have not gone through the full approval process (which includes approval by a change board of business users) because of security issues. Due to a continual backlog, it will be about 2-5 years before they would be approved... As to editors, that's a whole other bag of laughs.
Dan McGrath
I was trying to give you the benefit of the doubt, but you sound like you're just begging for us to tell you to "jump ship, now". :-)
Ken
I was hoping for something insightful I hadn't thought of, as well as generating a place where I could reference statements other than my own and colleague's in regards to the situation. Part of the "It isn't just my opinion" method. Which, for the record, did result in the development work being done in the correct way with positive results. It should be noted that we haven't had this particular issue since, so although the place isn't perfect, change for the better does happen.
Dan McGrath
Python is good for text processing.
Hamish Grubijan
+1  A: 

The underlying problem here seems to be that you have 'analysts' who describe, presumably necessary, functional changes in terms of adding a goto to some code. And then you have 'programmers' whose job appears to be limited to typing in that change and then complaining about it.

To make anything different, that dysfunctional allocation of responsibilities between those people needs to change. There are a lot of different ways to split up the work: The classic, conventional one (that is quite likely the official, but ignored, policy in your work) is to have analysts write a implementation-independent specification document and programmers implement it as maintainably as they can.

The problem with that theoretical approach is it is actually unworkably unrealistic in many common situations. In particular, doing it 'properly' requires employees seen as junior to win an argument with colleagues who are more senior, have better social connections in the office, and more business-savvy. If the argument 'goto is not implementation-independent, so as an analyst you simply can't say that word' doesn't fly at your workspace, then presumably this is the case.

Far better in many circumstances are alternatives like:

  1. analysts write client-facing code and developers write infrastructure.
  2. Analysts write executable specifications which are used as reference implementations for unit tests.
  3. Developers create a domain specific language in which analysts program.
  4. You drop he distinction between one and the other, having only a hybrid.
soru
No, the problem is analysts who are able to 'see' a presumably faster way of achieving the same results in the short-term, which turn out to be flawed in regards to long-term maintainability (and as it turns out, in the short-term as well). The problems you describe are real problems, but secondary to the actual issue that needing resolving in this situation.
Dan McGrath
Well yes, that's the point: they are wrong, but noone has either the authority to tell them, or capability to persuade them.
soru