1 Code improvement recommendation
Robin van Boven edited this page 2015-01-24 20:46:52 +01:00

The code quality is rather poor at the moment. By improving this we can reduce the number of bugs and make it easier for new (and existing) programmers to develop for Friendica.

The way to do this is to introduce good coding practises and rewrite any poor code. So in this document I will suggest some recommendations.

Incremental changes

Nobody really has time to write the whole thing again from scratch. So the way to go about this is by making small changes at a time.

Change a small piece of the code, test it well and submit a pull-request. All of this without breaking existing functionality, but it improves the code.

It was like that when I got here: Steps towards modernizing a legacy codebase is a great talk on this subject.

Simple code is better code

You can read the full article about simplicity-oriented design. But here is the short version.

Simple code:

  • Is easy to understand, so it will have fewer bugs.
  • Will solve a problem quickly, so you can develop faster.
  • Is a good quality measure. Simple is better than complex.

Feedback is important

When new code is introduced or when code is changed, be sure to comment on the quality of the code. If there are things that can be improved, let the author know in a constructive way.

A great moment to do this for example is when looking at a pull-request.

Using Git(Hub)

We are using Git and GitHub, but lets explore some good practises.

Create a branch when you do any work

Whenever you start something new, you should make a branch for it. Give it a useful name, like hotfix/bug-1234 or feature/new-facebook-api.

This makes sure that you are always able to pull the latest changes without breaking your changes. And the other way around, you can make sure your changes will merge cleanly when you are ready to make a pull request.

For more advanced branching, read about Git flow.

Do not accept your own code

Both maintainers and contributors should create pull requests for their changes even if you have push access to the repository. When you've made a pull request, let someone else look at your changes and have them accept/reject it. This way, if there's a visible problem with your code, it is possible to fix it before going into the main repository.

Create an issue for every bug

When you fix a bug, there should always be an issue to describe the bug. Even if you made a commit to fix it. Others need to know what you've fixed.

Create an issue for new functionality too

Why? Because you should be able to explain to others: why this functionality is needed. This is especially true for large pieces of new functionality.

  • It prevents bloating the system.
  • It makes sure you don't build something that won't be used.

Object-oriented programming (OOP)

This is a MUST USE practise. All of the code (excluding perhaps index.php or cli.php) must use OOP structures. If you need to modify code that doesn't use OOP, consider rewriting it. If you need to write new code, always use OOP.

We assume you are familiar with the basics of OOP, if not here are some resources.

Furthermore we have some recommendations to do this properly. Following these recommendations will greatly improve the quality of your code.

Use namespaces (PSR-4)

Namespaces are really simple and really helpful. It's like using folders/directories on your file system.

Always use them. You should never need to add anything to the global scope.

An example namespace may look like this: Friendica\Core\Protocols\Diaspora.

  • Created by the vendor Friendica.
  • Inside the Core package (in other words, this is the Friendica core).
  • We have a collection of Protocols.
  • Which has Diaspora as one of the protocols.

Read more on PSR-4

Create small classes

One class is responsible for one task.

If you have large classes (say, 300+ lines of code), you should split it into multiple classes. Try to identify any sub-tasks that are being implemented and move them to a class of their own.

Throw exceptions

Something went wrong? Don't use die() or echo to output this in production code. Throw an exception. If it is not caught that will cause the code to stop with great debugging information.

Use dependency injection

This is not so complicated as it sounds.

Do not hard-code where to get your dependencies from, set them using the constructor or setters.

This is very important for classes that require automated testing. Without dependency injection it's almost impossible to write good unit tests.

class BadExample
{
  protected $iNeedThis;
  
  public function __construct()
  {
    // This is wrong.
    $this->iNeedThis = new HardCodedClass();
  }
  
}

// We don't know and can't change that BadExample uses the HardCodedClass here.
$example = new BadExample();

class GoodExample
{
  protected $iNeedThis;
  
  public function __construct(InjectedClass $iNeedThis)
  {
    // Not hard-coded! :D
    $this->iNeedThis = $iNeedThis;
  }
}

// Now we know what the dependencies are and we can influence it!
$dependency = new InjectedClass();
$example = new GoodExample($dependency);