DZone
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
Refcards Trend Reports Events Over 2 million developers have joined DZone. Join Today! Thanks for visiting DZone today,
Edit Profile Manage Email Subscriptions Moderation Admin Console How to Post to DZone Article Submission Guidelines
View Profile
Sign Out
Refcards
Trend Reports
Events
Zones
Culture and Methodologies Agile Career Development Methodologies Team Management
Data Engineering AI/ML Big Data Data Databases IoT
Software Design and Architecture Cloud Architecture Containers Integration Microservices Performance Security
Coding Frameworks Java JavaScript Languages Tools
Testing, Deployment, and Maintenance Deployment DevOps and CI/CD Maintenance Monitoring and Observability Testing, Tools, and Frameworks
Culture and Methodologies
Agile Career Development Methodologies Team Management
Data Engineering
AI/ML Big Data Data Databases IoT
Software Design and Architecture
Cloud Architecture Containers Integration Microservices Performance Security
Coding
Frameworks Java JavaScript Languages Tools
Testing, Deployment, and Maintenance
Deployment DevOps and CI/CD Maintenance Monitoring and Observability Testing, Tools, and Frameworks

Broken Windows: How to Create Clean Code

Code quality gets talked about a lot, but it's easier said than done. In this post, a senior developer talks about some ways to keep code clean.

Tomasz Waraksa user avatar by
Tomasz Waraksa
CORE ·
May. 02, 19 · Analysis
Like (2)
Save
Tweet
Share
6.46K Views

Join the DZone community and get the full member experience.

Join For Free

Introduction

This time it won't be my favorite rant about certain operating system. Instead, just a few thoughts about the psychology of bad code.

The broken windows theory, an academic theory proposed by James Q. Wilson and George Kelling in 1982, is a metaphor for disorder within neighborhoods. Their theory links disorder and incivility within a community to subsequent occurrences of serious crime. - Encyclopedia Britannica 

The Tale of Shrubbery Code

Once, in the past, me and my team had a discussion about event handlers in front-end code. We looked in bewilderment at some ancient code. Something similar to this Vanilla JS (I'm a big fan of Vanilla JS, to be clear):

    this.btnSave.addEventListener('click', () => {
      window.fetch('/selection', {
        method: 'POST', headers: { 'Content-Type': 'application/json' },
        body: JSON.stringify(this.items.map(item => { id: item.id, selected: item.selected })))
      .then(response => response.json())
      .then({ data, success, error } => {
        if (success) {
          Notification.info('Data saved succesfully');
          this.items = data.items;
        } else throw new Error(error);
      })
      .catch(error => {
        Notification.error(error.message);
        this.close();
      });
    });

This is a fine specimen of shrubbery code. One looks at it and has no clue how to approach it without hurting oneself. Maybe I should read it backwards? How about unit tests? Don't look at these either — it's equally bad code which proves nothing expect that code coverage is at 70% so everybody's happy.

We've discussed the following problem. A programmer was asked to add a Delete button. That's easy! Copy-paste, and... there are two shrubberies now. Then three. Then it's a mess. The only thing one can do at this point is rewrite. So what should I put into an event handler?

I said a single line is enough. There was serious disagreement.

The Rewrite

Here's an even more troubling question — why would a developer plant another shrubbery at all? Couldn't they see how bad it is already? This made me think. What if the initial code looked like this:

    // Submits HTTP request
    submitRequest (endpoint, data, method = 'POST') {
      const body = data ? JSON.stringify(data) : undefined;
      const headers = { 'Content-Type': 'application/json' },
      const options = { method, headers, body };
      return fetch(endpoint, options)
        .then(response => response.json());
    }

    // Handles received response
    handleResponse (operation, { data: { items }, success, error }) {
      if (success) {
        Notification.info(`${operation} succeeded`);
        return items;
      } else throw new Error(`{$operation} failed`);
    }

    // Handles request error
    handleError (error) {
      Notification.error(error.message);
      this.close();
    }

    // Returns current selection
    getSelection () {
      return this.items.map({ id, selected } => ({ id, selected }));
    }

    // Submits SAVE SELECTION request
    submitSelection () {
      const endpoint = '/selection';
      const data = this.getSelection();
      return this.submitRequest(endpoint, data)
        .then(response => this.handleResponse('Save', response));
    }

    // Updates current selection in UI
    refresh (items = []) {
      this.items = items;
    }

    // Handles save button click
    saveButtonClicked () {
      this.submitSelection()
        .then(items => this.refresh(items))
        .catch(error => this.handleError(error));
    }

    // Wires up events
    this.saveButton.addEventListener('click', () => this.saveButtonClicked());

It's far from ideal. There are 10 other ways to structure it. Request handling code sure will be moved to a separate service. But it's a good start. It reads easy. The event handler now just has one line of code, where it all begins. From here I can drill down into the details. Unit tests for these little functions are easy to write.  Another handler for the delete button would be trivial, as it would reuse most of the code.

It gets even better! If I read it backwards, it still makes perfect sense!

Yes, it is more lines of code. But since we're not paid per line-count anymore, this should no longer be an issue for the management. Meanwhile, I can read it and learn in small bits, without an instant brain seizure. I want to write this kind of code!

That was the answer. If the developer in question saw the above, they'd follow the pattern. Unless they wouldn't, but then we have a team problem.

So Why Isn't All the Code Neat Like That?

We all have our coding guidelines, code reviews, books of standards, John Papa, and Todd Motto. Yet we keep churning out shrubbery code. Why?

Guidelines and standards are nice. But we always need to apply them. We should ignore remarks like these and do our job properly:

"Look, this little component has just two event handlers, they're a few lines each and will not be reused elsewhere, so why not keep it concise?"

First, we often mistake concise for compressed. They're not the same. Check https://www.ioccc.org/ for proof.

Finally, the broken windows theory. It's not just about me. It's about everybody else who will someday work on this code, including a future me. How?

Make it a miserable place and whoever comes after me will keep adding to the misery. If I encounter a shrubbery... well, why would I be a hero and get rid of it? Just fix what needs a fix and get the hell out of there. And think twice before cleaning it up. Such heroism usually backfires, as a team instantly assumes that from now on I'm the one responsible for this whole mess.

Put in place a disciplined structure, separate the concerns, apply rules consistently, have JSDoc comments, and proper unit tests — and they will follow. Because it feels wrong to spoil such an idyllic landscape!

For inspiration, here's a great Daniel Stori comic!

Image title

code style

Opinions expressed by DZone contributors are their own.

Popular on DZone

  • 9 Ways You Can Improve Security Posture
  • 13 Code Quality Metrics That You Must Track
  • What Are You Missing by Debugging in VS Code?
  • How To Get Page Source in Selenium Using Python

Comments

Partner Resources

X

ABOUT US

  • About DZone
  • Send feedback
  • Careers
  • Sitemap

ADVERTISE

  • Advertise with DZone

CONTRIBUTE ON DZONE

  • Article Submission Guidelines
  • 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: