Development: Hints for Refactoring Existing Drivers: Difference between revisions

From LinuxTVWiki
Jump to navigation Jump to search
No edit summary
No edit summary
Line 96: Line 96:


* Sometimes a few mails refining the final details close the discussion.
* Sometimes a few mails refining the final details close the discussion.

Has happened multiple times in the past, see the LKML and linux-dvb mailing list archives for some amusing references.





Revision as of 10:53, 30 September 2004

Introduction

Free software grows over time, many people are involved in development and not always the original authors are still available or willing to maintain the code forever. So then and when the code has to get cleaned up, Refactored to make it future-proof. You don't need to be core-developer to do so, everybody is invited to review the code, join development and contribute.

Basically Code Review can be done on very different levels: just check the code for correctness and perform Bugfixes, introduce Performance Optimisations or, Refactoring.


What is Refactoring?

You find a short definition of the Refactoring Process on Wikipedia, mentioning the important facts. The basic principles are:

  • Simplify your Code.
  • Don't introduce new Features in the Refactoring Process.
  • Break down API Barriers where they don't make Sense
  • Introduce new Interfaces where they stabilize your Code.
  • Try to keep external Interfaces as stable as possible.
  • Split complex Modules into smaller, easily testable ones with well-defined Tasks.
  • Avoid Confusion.
  • Simplify your Code.

Refacturing is a natural process, usually projects grow over time and then and when this naturally grown jungle needs to get urbanized again to make it a nice place to live. This process is usually accompanied by (sometimes loud and rude) discussions about Philosophy and holy Wars, but don't worry: dogs that make noise usually don't bite. Revolutionary ideas sometimes need some fighting to get through.

Since with time developers get a little blind and used to their own weaknesses it makes sense that reviewer/refactorer and programmer are not the same person. No part of the Linux kernel is exclusively owned by a maintainer. If a new solution makes sense it will make it sooner or later into the public source tree.

Refactoring does not means new features. Refactoring means that you make the code maintainable.


Keep it Simple

The simpler the resulting code is the easier it will be to maintain. Even from the performance point of view simple code is almost always preferable.

If you want to see some impressive code following this approach you may want to check out one of Fabrice Bellards Projects, e.g. TinyGL or the ID software GPL games Doom, Quake & Co Great to learn performant and compact Coding.

So please: Whatever you do, please keep it Simple.


Please do never ever apply big Changes in-place

If you plan to rewrite bigger portions of the code please don't create a huge patch or thousands of patches but fork the relevant code modules so that people can easily test them concurrently with the old code and are free to use the old, usually well-tested code until your new version has matured.

There have been several situations in the past where the linux-dvb CVS was barely usable for weeks or months because we did not followed this principle. Please respect that other people are using the linux-dvb source in productive environments and rely on a working code base.

Please avoid postings like "[PATCH] I ported all drivers to this and that kernel API because this is the way the kernel maintainers like it to see!". Sometimes people are going their own way with reason, you are never able to foresee the effect of such major changes on all the exotic platforms the linux-dvb stack is used. If you really think using the new API is the way to go then please let's fork the source tree and move over to the new tree as soon it is verified on all architectures and well-tested.


Psychological Prerequisites

Refactoring does not only means to rewrite code but also to convince all other developers to accept the changes and to join your efforts.


Approach #1: The Democratic Trial -- let's discuss!

May sound very appealing (and as if it would save much useless work that won't get accepted later if you ask in advance), but please keep in mind that these endless threads cost a lot of energy, the accompanying flame wars on the related mailing lists long often several weeks, and have often happening like this:

  • Somebody starts the discussion and makes his prosaic proposal.
  • nothing happens for a while.
  • Guru #1 says: "mmhh... interesting!"
  • Guru #2 says: "No go!!! we need to do it like I already started years ago!!"
  • Guru #3 says: "yes, Guru #2 is right, we have to follow the mainstream API development!"
  • Guru #1 annotates that the new approach has some appealing properties and simplifies many things a lot.
  • several less involved developers join discussion and take sides with the new Guru #1, #2 or Guru #3.
  • The Proposer refines his approach and maybe even provides some sample code.
  • Guru #1 says that he likes it.
  • Guru #2 or Guru #3 make their own proposal and post some sample code explaining the plan they are following since years.
  • Guru #1 calls it bloat and emphasizes how simple the new approach may be.
  • Guru #3 says that we would have to rewrite a lot of code.
  • (mails get harsh in sound, sometimes even quite personal. Arguments count less and less.)
  • The Proposer, Guru #2 and Guru #3 post several new proposals converging closer to each other. The sample code gets more and more complicated due to all the special cases that come to discussion.
  • For a while it becomes quiet.
  • Guru #2 or Guru #3 posts a new approach that funnily looks quite similar in concept to the original code of the proposer but is a little less clean due to the exhausting fights.
  • Proposer says: "hey, that looks familiar!" -- Guru: "may be."
  • Sometimes a few mails refining the final details close the discussion.


Approach #2: Just do it

The process outlined above may sound very democratic and entertaining but it eats a lot of time and good mood, is thus kind of counterproductive. If you don't want to spend your energy in flame wars you may try this:

  • Implement your approach for 2 or 3 sample drivers (choose a complex one, e.g. a STB driver or the av711x-ttpci driver and 2 simple ones, a hotplug-USB device and a modern PCI card).
  • Post your code on the linux-dvb mailing list and propose to start a new development branch in CVS where this work can get continued. Request a CVS account if you don't already have one so that you can easily work on your code.

If your new code has serious advantages it will get incorporated for sure. In most cases it will take less time to write the new code than to await the end of discussion.

DVB driver development is done by everybody who wants to join development, no Guru has absolute power over the code. So you can be sure that everything that makes sense will make it sooner or later into the mainstream source tree.

Additionally, you achieved a nice side effect: Every developer will be highly motivated to make his own driver as elegant as your reference code, so you can be sure that all developers will join the development. This is in opposition to the "Democratic Discussion Process" where the risk is high that first many developers are pissed off and later try to "personalize" the code just because they can't resist to put in some pieces of their old theory just to keep it alive. And nobody asks why they're doing it because everybody fears a new flame war. This prevents emergence of homogenous and easy-to-maintain code.