DEV Community: Ryan Silva

3 Steps to DRY

Ryan Silva — Wed, 30 Aug 2017 23:57:03 +0000

Don’t Repeat Yourself: it’s a crucial refrain to remember. Avoiding repetition in your codebase saves development time and improves maintainability, but getting there isn’t always easy. Repetition can come in many forms, from a copy/pasted low level function to multiple similar UI components with separate implementations. I’ll focus on the latter as I present three steps that help me DRY.

1. There Is No Pattern

Before obsessing about DRY, don’t forget its counterpart, YAGNI: Ya Ain’t Gonna Need It. The quest for DRY can lead to over-engineering: added complexity to handle a pattern that never emerges. While smart engineering is always flexible, be wary of adding large costs to enable reuse if you think that reuse may never happen.

2. Establish a Pattern

If there is a pattern in the system, let it emerge. How long you wait depends on your team and your system, but always actively encourage design details that move the overall design towards one that can be abstractly implemented. This also ensures a consistent experience for your users.

3. Code the Pattern

Once a pattern has been established, code the pattern. Don’t wait too long! At this point the cost of creating a reusable component has obvious benefits, and you want to take advantage of that benefit when the next feature comes along. If you wait too long, you pay double the cost: the initial cost of repeating yourself, and the cost of refactoring it later.

Case Study: “Category Table”

A few years ago I started a new application and I needed a table-like component with rows representing categories that expanded to reveal a form. There were a few of these tables, but all different. I wrote CSS that could be reused between the tables, but I didn’t attempt to write a single component for these “category tables”. I knew that the design was in flux and that making it generic enough would introduce a lot of complexity.

Eventually it became clear that we would use this category table in many places. So when new features were designed using a variant of the table, I ensured that the new iterations were consistent with requirements of the old tables, laying the foundation for a reusable implementation.

As soon as we could see that the different tables in our app were really instances of the same thing, my colleague created a Javascript component that was flexible enough to work for all of them. Then I refactored all the existing tables to use the new component. New category tables continued to appear, but we were able to continue using the reusable component, saving many days of development time.

Playing the API We're Dealt

Ryan Silva — Mon, 13 Feb 2017 16:34:50 +0000

It's not about the API you're dealt, but how you play the hand

When writing web applications, sometimes we aren't dealt the API we want. Either the "RESTful" APIs are anything but, or the data is sent in a weird format, or we might have to support requests that don't fit into a REST model. When we can't change our hand, our job is to make the UI code functional without becoming an unmaintainable mess.

My approach to working with the API follows the "thin controllers, fat services" methodology. I want all business logic and data manipulation to happen in a service, abstracted behind sensible, predictable, and easily testable methods. Following are some ways that I've played my not-quite-ideal hand. I'm using Angular, but these ideas are not framework-specific.

Mixing Up the Payload

The most common situation I encounter with my APIs is that I want to change the payload structure. Sometimes I want to initially sort a collection because the API wouldn't sort it for me, or I simply need to rename a property. In other cases, I've needed to extract a completely structure from the API response.

To address this, I borrowed a convention from BackboneJS and I implement a parse method, which takes raw response data and returns the object's desired data. I have a common REST service which uses this method. Everyone working on our codebase knows that any incoming data manipulation happens in parse, which provides predictability and consistency. With clear input and output requirements, parse is also easy to unit test.

The logic in parse should never go in the controller. We want the data representation to be altered only once when it arrives and remain consistent throughout different views. If one controller rearranges the data, then we risk regressions if that controller is ever removed and other code depended on the new format.

The following snippet shows the service, controller, and unit tests for a basic API call that needs some data sorted and a field reassigned after being fetched:

//MyResource will store the data fetched from the API
class MyResource {
  get url() {
    return '/api/my_resource';
  }
  //Incoming data will have its list sorted and a description field set
  parse(data) {
    if (data) {
      if (data.list) {
        data.list = data.list.sort();
      } else {
        data.list = [];
      }
      data.description = data.notes;
    }
    return data;
  }
}

//An abbreviated example of a common REST service
function restServiceFactory($http) {
  class RestService {
    //Perform a GET of the resource
    fetch(resource) {
      return $http.GET(resource.url).then(resp => {
        //First run the data through parse
        let data = resource.parse(resp.data);
        //Is this a collection? Update the array with the response data
        if (isArray(resource)) {
          resource.splice(0, resource.length, ...data);
        } 
        //It's an object; update it with the response data
        else {
          angular.extend(resource, data);
        }
        return resource;
      });
    }
  }

  return new RestService();
}

angular.module('my.module', []).factory('restService', restServiceFactory);

//Example controller which fetches MyResource
class MyController {
  constructor(restService) {
    this.resource = new MyResource();
    restService.fetch(this.resource);
  }
}

//Unit tests for MyResource
describe('MyResource test', function() {
  it('initializes list', function() {
    let resource = new MyResource();
    expect(resource.parse({}).list).toEqual([]);
  });

  it('sorts the list', function() {
    let resource = new MyResource(),
      list = ["pear", "apple", "banana"];
    expect(resource.parse({list}).list).toEqual(["apple", "banana", "pear"]);
  });

  it('sets description field from notes', function() {
    let resource = new MyResource(),
      notes = "some notes";
    expect(resource.parse({notes}).description).toEqual(notes);
  });
});

Occasionally I need to extract an entirely different structure than what the API provides. Let's say I have a book list API. My app actually needs a list of authors, but that API doesn't exist. I might be tempted to do something like this:

class BookCollection extends Array {
  get url() {
    return '/api/books';
  }
}

class AuthorListController {
  constructor(restService) {
    //Fetch list of books, then extract the author list from it
    restService.fetch(new BookCollection()).then(books => {
      let authorsMap = {};
      this.authors = [];
      books.forEach(book => {
        if (!authorsMap[book.author.id]) {
          authorsMap[book.author.id] = true;
          this.authors.push(book.author);
        }
      });
    });
  }
}

But now the controller is fatter, and this logic can't be reused by other views. This would be far better:

class BookCollection extends Array {
  get url() {
    return '/api/books';
  }
  getAuthors() {
    let authorsMap = {}, authors = [];
    this.forEach(book => {
      if (!authorsMap[book.author.id]) {
        authorsMap[book.author.id] = true;
        authors.push(book.author);
      }
    });
    return authors;
  }
}

class AuthorListController {
  constructor(restService) {
    restService.fetch(new BookCollection())
      .then(books => this.authors = books.getAuthors());
  }
}

Now the logic is hidden in the service, and anyone with a book collection can get the authors list. But what if the app is primarily focused on authors, and we don't ever need a book collection? By using the parse method, we can change the service to better express our objective—we can deal with a collection of authors from the beginning:

class AuthorCollection extends Array {
  get url() {
    return '/api/books';
  }
  //API input is a list of books
  parse(data) {
    let authorsMap = {}, authors = [];
    this.forEach(book => {
      if (!authorsMap[book.author.id]) {
        authorsMap[book.author.id] = true;
        authors.push(book.author);
      }
    });
    return authors;
  }
}

class AuthorListController {
  constructor(restService) {
    restService.fetch(new AuthorCollection())
      .then(authors => this.authors = authors);
  }
}

Now the controllers can deal solely with author collections, and the fact that it has to use a different API to get the data is completely transparent.

Custom or Non-RESTful Requests

The previous examples show how to manipulate data on basic GETs—they can all use the same fetch method to perform the request. But what if the request is non-RESTful, uses an unexpected verb, or needs to be customized in some special way? I extend the default RestService as a new service and add or override methods as needed. The service should always have readable, obvious methods that describe what they're going to do.

Overriding Service Methods

If we're already using fetch to do GETs, then it makes sense to override fetch if we need to do something different when downloading a resource. For example, if I want to get the list of books, but I want to use a query parameter to sort the collection first:

class RestService {
  //fetch accepts httpOptions 
  fetch(resource, httpOptions) {
    return $http.GET(resource.url, httpOptions).then(/*...*/);
  }
}

class BookService extends RestService {
  //Override the default fetch, passing sortBy=name query parameter
  fetch(resource) {
    return super.fetch(resource, {params: {sortBy: 'name'}});
  }
}

class BookListController {
  constructor(restService) {
    restService.fetch(new BookCollection())
      //Already sorted by name
      .then(books => this.books = books);
  }
}

How about when the API developer insists that updating a book should be done as a POST to /books instead of a conventional PUT to /books/{id}? You can imagine that RestService has a save method which normally does a PUT to the resource URI. So we override save:

class RestService {
  //Typically, save does a PUT to the resource URI
  save(resource) {
    return $http.PUT(resource.url, resource).then(/*...*/);
  }
}

class BookService extends RestService {
  save(book) {
    return $http.POST(book.collectionUrl).then(/*...*/);
  }
}

class BookController {
  /*...*/
  save() {
    this.restService.save(this.book);
  }
}

There are many possibilities for customization, but the main point is the controller can be ignorant of the fact that this endpoint is weird. It just calls the same save method that it always calls.

Custom Service Methods

Sometimes we need to do something that isn't RESTful and doesn't fit into our model. Suppose a book URI includes the author: /authors/{authorId}/books/{bookId}. We want to change a book's author, which means a new book URI and some updated author collections. Our API provides a separate endpoint to reassign books to new authors: a POST to /authors/{authorId}/books/{bookId}/reassign. We can handle this with a new service method:

class BookService extends RestService {
  reassign(book) {
    return $http.POST(`${book.url}/reassign`).then(/*...*/);
  }
}

Similar to the custom save and fetch, this new method provides new functionality for a book. Any complexity involved in reassigning a book should be hidden here, so the controller doesn't need to know about it.

When we have to play the API we've been dealt instead of the API we want, it takes some thought to keep the UI code in good shape. By confining all special API logic in the service layer, we keep the controllers simpler, reduce regressions, and maintain testability. What are some ways you've played your hand?

Land Mines

Ryan Silva — Tue, 31 Jan 2017 22:57:36 +0000

Several years ago, I got the call every engineer dreads: there's a big problem in production. The issue manifested itself dramatically but was caused subtly. I had made significant performance improvements by altering a SQL SELECT * query to exclude a column containing a large blob of text. That text wasn't used in that page... or so we thought.

During the refactor, I identified that text was used in one check, but after conferring with other engineers we decided it was OK to remove the field in this query anyway. It turns out that check was important. At some point years prior, the team decided that the presence of a certain substring in the large text blob would affect the value of a very important boolean field in a non-obvious way. Excluding that text from the SQL query, in certain cases, affected this important boolean value.

More recently, I noticed a different issue. Our API applied a default setting based on create date, because on a certain date the default settings changed. Seven months later, a change to the way we used a 3rd party library affected how we set create dates, which then affected the settings on very specific older data.

These decisions—changing a boolean based on a keyword in a large blob of otherwise unnecessary text, applying a setting based on create date—are land mines. They fix an issue at hand and lie silent for months or years only to explode when an unsuspecting developer makes a routine change.

Nobody likes land mines, but how do you prevent them?

Avoid Shortcuts

This may sound obvious, but it's so easy to take a shortcut and move on. In fact, that's probably the token signature of a land mine: a questionable decision made in the interest of time that solves the problem... for now. But please, think about future you, future me.

Be Obvious

When writing business logic, think about the inputs to your function. Would another developer reasonably expect change of input data to affect the output of your function? Try to imagine scenarios where your input data might change. Would this affect the output in unexpected ways? I don't think most people would expect settings to change based on the create date.

Consider Migration

In both of the instances above, a "dirty hack" was introduced to avoid running some kind of migration on the existing data. Migration is scary, I get it. But the outcome of avoiding a migration can be scarier. Migration is a tool in your toolbox that you should not be afraid to use. When weighing options, remember to weigh the obvious risk from migration against the not-so-obvious risk of introducing code complexity and possible land mines for future developers.

I hope these tips will help keep your codebase clear of land mines for all of us.