Last minute geek

last minute tech news from around the net

Sunday, Nov 19th

Last update01:00:00 AM

You are here: English WTF CodeSOD: Drain the Swamp

CodeSOD: Drain the Swamp

User Rating: / 0
PoorBest 

You may remember Virginia N from An Extinction Event, where she struggles to refactor a legacy project with some… unusual design principles. ReSharper still continues to choke to death on their codebase, but her management haas let her know, this won’t be a problem going forward.

“You see,” her boss explained, “we’re going to move the logic into stored procedures. That way, we can more easily re-use the logic between the Windows Forms client and the Web app.”

“Oh, there’s going to be a web app, now?” Virginia asked.

“Yes! They’re going to use best practices, like unit testing, so that they don’t end up with the same kind of mess we have,” her boss said.

Virginia was halfway to filing a request for a transfer when she heard more about the web project. Then she heard about their plan. They weren’t going to simply build a web-client for their backend, but instead were going to build an inner platform. The page logic would be a simple template, and all of the rules, styling, data and display logic would be stored as data in the database. “It’s gonna be really flexible!”

Virginia decided to stick with the fetid field she knew, instead of the “green field” which was going to be a fetid swamp in a matter of weeks.

In Virginia’s swamp, she has many, many 40,000 line classes. That much code means the code has no real cohesion, so it leads to Virginia finding variables named thus:

public bool IReallyDontWantToFetchTheDataOnLoadButDontWantToChangeTheOtherVariablesCauseWhoKnowsWhatWillHappen=false;

It’s at least descriptive. It also exists in the class side-by-side with variables like blnGetData, GetDataOnFormLoad, and GetDataOnFormrLoadS.

Elsewhere in the same file, they have a different problem. They added a control to the view, and needed an accessor method to decide whether it was visible or not. Actually, they needed a few, and they knew that the form would be changing over time, so they needed something that was “dynamic”.

Now, they could have simply added properties and getters/setters as the form changed and dealt with the follow on changes, but someone decided it was time to stress “Closed for Modification” was a good object-oriented practice. They left out the “Open for Extension” part of the open/closed principle, so they used this code instead, which uses a few index properties to decide which control should be modified.

private void SetVisiblePicNdt()
{
    string NomBtnNDT="";
    string NomBtnChant="";
    if (m_IndexChant>0)
    {
            NomBtnNDT="btn"+(m_IndexChant+1).ToString();
            NomBtnChant="btn"+(m_IndexChant).ToString();
    }
    else
    {
            NomBtnNDT="btn"+(m_IndexLB+1).ToString();

    }

    Control sectMain=null;

    for (int i=0;i<ctlRecherche.Controls.Count;i++)
    {
            if (ctlRecherche.Controls[i].Name=="sectMain")
            {
                    sectMain=ctlRecherche.Controls[i];
                    break;
            }

    }
    if (sectMain!=null)
    {
            for (int i=0;i<sectMain.Controls.Count;i++)
            {
                    if (sectMain.Controls[i].Name=="panFilterSection")
                    {
                            panFilterSection=sectMain.Controls[i];
                            break;
                    }

            }
    }
    if (panFilterSection!=null)
    {
            for (int i=0;i<panFilterSection.Controls.Count;i++)
            {
                    if (panFilterSection.Controls[i].Name==NomBtnNDT)
                    {
                            panFilterSection.Controls[i].Visible=false;
                            break;
                    }

            }
    }

}

Virginia has many more horrors to share, so expect more examples from this code base.

[Advertisement] Release! is a light card game about software and the people who make it. Play with 2-5 people, or up to 10 with two copies - only $9.95 shipped!

Read all
Comment Policy:
We pre-moderate any comments and welcome all kinds of thoughts, supportive, dissenting, critical or otherwise. We delete or censor comments that are:

* abusive
* off-topic
* contain personal attacks, or against any company or organization
* promote hate of any kind
* use excessively foul language
* is blatantly spam or advertising

We do not discriminate based on the person who is posting, and we never censor comments for political or ideological reasons. We never delete an appropriate comment because we disagree with its viewpoint or ideology, and we never publish an inappropriate comment because we agree with or support its viewpoint or ideology.


Attention spammers: we manually approve all comments. Spamming and blatant advertising will NOT be published on this site and is deleted immediately, you've been warned, do not waste your time here.

Add comment

Security code
Refresh