How to write a good and quality code?

Build solid, not stupid code

Let’s start by introducing the most elementary rules that are called SOLID. It is a term describing a collection of design principles for good code that was invented by Robert C. Martin and how it goes:

S means Single Responsibility Principle

It states that a class should have one and only one reason to change. In other words, a class should have only one job to do, so we should avoid writing big classes with many responsibilities. If our class has to do a lot of things then each of them should be delegated into separate ones.

class Notification
  def initialize(params)
    @params = params
  end

  def call
    EmailSender.new(message).call
  end

  private

  def message
    # some implementation
  end
end

O means Open/Closed Principle

It states that you should be able to extend a classes behavior, without modifying it. In other words, it should be easy to extend a class without making any modifications to it. We can achieve this e.g by using strategy pattern or decorators.

class Notification
  def initialize(params, sender)
    @params = params
    @sender = sender
  end

  def call
    sender.call message
  end

  private

  def message
    # some implementation
  end
end

class EmailSender
  def call(message)
    # some implementation
  end
end

class SmsSender
  def call(message)
    # some implementation
  end
end

With this design, it is possible to add new senders without changing any code.

L means Liskov Substitution Principle

It states that derived classes must be substitutable for their base classes. In other words, usage of classes which come from the same ancestor should be easy to replace by other descendant.

class Logger {
  info (message) {
    console.info(this._prefixFor('info') + message)
  }

  error (message, err) {
    console.error(this._prefixFor('error') + message)
    if (err) {
      console.error(err)
    }
  }

  _prefixFor (type) {
    // some implementation
  }
}

class ScepticLogger extends Logger {
  info (message) {
    super.info(message)
    console.info(this._prefixFor('info') + 'And that is all I had to say.')
  }

  error (message, err) {
    super.error(message, err)
    console.error(this._prefixFor('error') + 'Big deal!')
  }
}

We can easily replace the name of the class, because both has exactly the same usage interface.

I mean Interface Segregation Principle

It states that you should make fine grained interfaces that are client specific. What is an interface? It’s a provided way of usage of some part of the code. So a violation of this rule could be e.g a class with too many methods as well as a method with too many argument options. A good example to visualize this principle is a repository pattern, not only because we often put a lot of methods into a single class but also those methods are exposed to a risk to accept too many arguments.

# not the best example this time
class NotificationRepository
  def find_all_by_ids(ids:, info:)
    notifications = Notification.where(id: ids)
    info ? notifications.where(type: :info) : notifications
  end
end

# and a better one
class NotificationRepository
  def find_all_by_ids(ids:)
    Notification.where(id: ids)
  end

  def find_all_by_ids_info(ids:)
    find_all_by_ids(ids).where(type: :info)
  end
end

D means Dependency Inversion Principle

It states that you should depend on abstractions, not on concretions. In other words, a class that uses another one should not depend on its implementation details, all what is important is the user interface.

class Notification
  def initialize(params, sender)
    @params = params
    @sender = sender
  end

  def call
    sender.call message
  end

  private

  def message
    # some implementation
  end
end

All we need to know about sender object is that it exposes `call` method which expects the message as an argument.

Not the best code ever

It is also very important to know the things which should be strictly avoided while writing code, so here goes another collection with STUPID principles which makes code not maintainable, hard for test, and reuse.

S means Singleton

Singletons are often considered as anti-patterns and generally should be avoided. But the main problem with this pattern is that it is a kind of excuse for global variables/methods and could be quickly overused by developers.

T means Tight Coupling

It is preserved when a change in one module requires also changes in other parts of the application.

U means Untestability

If your code is good, then writing tests should sound like fun, not a nightmare.

P means Premature Optimization

The word premature is the key here, if you don’t need it now then it’s a waste of time. It is better to focus on a good, clean code than in some micro-optimizations - which generally causes more complex code.

I mean Indescriptive Naming

It is the hardest thing in writing good code, but remember that it’s is not only for the rest of your team but also for future you, so treat you right :) It is better to write a long name for a method but it says everything, than short and enigmatic one.

D means Duplication

The main reason for duplication in code is following the tight coupling principle. If your code is tightly coupled, you just can’t reuse it and duplicated code appears, so follow DRY and don’t repeat yourself.

It’s not really important right now

I would like to also mention two very important things which are often left out. You should have heard about the first one - it’s YAGNI which means: you aren’t gonna need it. From time to time I observe this problem while doing code review or even writing my own code, but we should switch our thinking about implementing a feature. We should write exactly the code that we need at this very moment, not more or less. We should have in mind that everything changes very quickly (especially application requirements) so there is no point to think that something someday will come in handy. Don’t waste your time.

I don’t understand

And the last thing, not really obvious I suppose, and may be quite controversial to some, it’s a descriptive code. I don’t mean by that only using the right names for classes, variables or methods. It is really, really good when the whole code is readable from the first sight. What is the purpose of the very short code whereas it is as enigmatic as it can be, and no one knows what it does, except the person who wrote it? In my opinion, it is better to write some chars_condition statements_something else more than one word and then yesterday sitting and wondering: wait what is the result, how it happened, and so on.

const params = [
  {
    movies: [
      { title: 'The Shawshank Redemption' },
      { title: 'One Flew Over the Cuckoo\'s Nest' }
    ]
  },
  {
    movies: [
      { title: 'Saving Private Ryan' },
      { title: 'Pulp Fiction' },
      { title: 'The Shawshank Redemption' },
    ]
  }
]

// first proposition
function uniqueMovieTitlesFrom (params) {
  const titles = params
    .map(param => param.movies)
    .reduce((prev, nex) => prev.concat(next))
    .map(movie => movie.title)

  return [...new Set(titles)]
}

// second proposition
function uniqueMovieTitlesFrom (params) {
  const titles = {}
  params.forEach(param => {
    param.movies.forEach(movie => titles[movie.title] = true)
  })

  return Object.keys(titles)
}

Both methods above do the same thing: fetch unique movie titles from quite complicated parameters. And there is a question: which one says clearer about how params argument is built and which one says clearer what we want to receive as a result? I leave the answer to your own reflection.

To sum up

As you can see, there are a lot of rules to remember, but as I mentioned at the beginning writing a nice code is a matter of time. If you start thinking about one improvement to your coding habits then you will see that another good rule will follow, because all good things arise from themselves just like the bad ones.

Vuelendar. A new Codest’s project based on Vue.js

Codest’s weekly report of best tech articles. Building software for 50M concurrent sockets (10)