DZone
Web Dev Zone
Thanks for visiting DZone today,
Edit Profile
  • Manage Email Subscriptions
  • How to Post to DZone
  • Article Submission Guidelines
Sign Out View Profile
  • Post an Article
  • Manage My Drafts
Over 2 million developers have joined DZone.
Log In / Join
  • Refcardz
  • Trend Reports
  • Webinars
  • Zones
  • |
    • Agile
    • AI
    • Big Data
    • Cloud
    • Database
    • DevOps
    • Integration
    • IoT
    • Java
    • Microservices
    • Open Source
    • Performance
    • Security
    • Web Dev
DZone > Web Dev Zone > Battle with legacy: reducing ifs

Battle with legacy: reducing ifs

Giorgio Sironi user avatar by
Giorgio Sironi
·
Aug. 14, 13 · Web Dev Zone · Interview
Like (0)
Save
Tweet
4.42K Views

Join the DZone community and get the full member experience.

Join For Free

Last week I navigated a 1500-line long function, which was revealed to be the culprite of a bug affecting a few countries which were recently switched from an older payment flow to a new, simplified one.

The function itself was littered with if() (and else and else if) statements, which we can easily smell as a problem to solve. But in this practical case, why do conditionals raise my concerns?

Some of the reasons are specifically related to legacy code.

Lots of logs

Ever-branching code is a phenomenon that causes the birth of extensive logging in a project.

Since you can never be sure what the code is doing:

if (...) {
  // 1
} else if (...) {
  // 2
  if (...) {
  // 3
  }
}

Did execution pass from 1, 2, or 3? Or in which combinations of them?

Given the difficulty of evaluating execution by hand, the only way to know in which branches an execution has entered is to log the hell out of this code:

if (...) {
  $this->log("1 is ready");
} else if (...) {
  $this->log("2 is ready");
  if (...) {
  $this->log("Entering 3");
  }
}

This logging has an overhead on performance, but over everything else on the understandability of the code. It's hard enough to follow a maze without having to strip the log calls that it contains and that separate the interesting pieces of functionality.

Difficulty to test

How many execution path the code shown previously can take?

  • passing only from 1
  • passing only from 2
  • passing from 2 and from 3
  • not passing from any of these

To these test cases that need to be exercised by 4 different test scenarios, add early returns to give the code the capability to execute just some of the code in a branch:

    if (...) {
        $this->log("Entering 3a");
            if (...) {
                return;
            }
        $this->log("Entering 3a");
    }

As the length of these procedures increases, it becomes impossible to test all the paths. But while segregating responsibilities in smaller pieces (such as objects or closures) creates a clear protocol between them, long procedures driven by conditionals have a big state space:

$answer = 42;
// 1500 lines of code
if ($answer === ..) {
  // still using that variable?
}

This enormous state space, containing all globals and local variables of the function, is the spine of the procedure and lets the execution use many combinations of the variables instead of encapsulating them into different places.

Difficult to extend

Finally, the ultimate problem with castles of conditionals is that they become impossible to extend. When you encounter a poorly tested castle, you tend to shy away from modifying it and assign responsibilities to other parts of the code. This can be a sensible strategy if you intend to absorb new requirements in different components and crush the long procedures later, but can perpetuate the problem as new legacy code is created because of the movement of responsibilities away from the right place for them to be handle (given the current requirements; there is no absolutely right way to implement a feature).

For example, consider this example with encrypted phone numbers: my current application is able to handle them in many different countries. The long, if-stained procedure would be the right place to handle the translation from an external crypted value to an internal standard database row.

However, since lions and other beasts were hidden inside the long procedure, some country-specific drivers had to become more complex to do the translation themselves (inserting the encrypted value in a database or searching it in case it was already known). This proved to be a problem, because it made the country-specfic code talk to a database it shouldn't know about: legacy code triggering the creation of other legacy code to compensare for its defects.

Conclusion

The combination of these factors caused the bug: the new payment flow was tested as a black box, but since this giant function was at the heart of legacy code it was left untouched when the first end-to-end scenarios became green. The specifications couldn't know about the complexity that has been hidden in that single place, because it grew to that size before the introduction of these tests. Discovering this bug in the test suite would have required an enormous amount of tests, to cover all the execution paths that the function could take.

Nothing new under the sun: legacy code has some good functional properties (it works for the existing use cases) but very bad non-functional ones: extensibility is forbidden due to regressions and the little understandability of the state space of the code, while testability costs too much. For every castle of conditionals a moment comes when it crumbles...

Opinions expressed by DZone contributors are their own.

Popular on DZone

  • PermGen and Metaspace
  • What Is High Cardinality?
  • Legacy Modernization and Hybrid Cloud with Kafka in Healthcare
  • Writing Beautiful, Optimized, and Better .NET Code With NDepend Static Analysis

Comments

Web Dev Partner Resources

X

ABOUT US

  • About DZone
  • Send feedback
  • Careers
  • Sitemap

ADVERTISE

  • Advertise with DZone

CONTRIBUTE ON DZONE

  • Article Submission Guidelines
  • MVB Program
  • Become a Contributor
  • Visit the Writers' Zone

LEGAL

  • Terms of Service
  • Privacy Policy

CONTACT US

  • 600 Park Offices Drive
  • Suite 300
  • Durham, NC 27709
  • support@dzone.com
  • +1 (919) 678-0300

Let's be friends:

DZone.com is powered by 

AnswerHub logo