Refactoring

From Dokeos

Jump to: navigation, search

Refactoring is an important, but often overlooked aspect of development. Without it code will quickly grow into an inconsistent mess, which leads to code bloat and increases probability of bugs. Refactoring can be used to make code easier to read, more flexible and easier to maintain.

Contents

Several small tips

  • Use long names for your variables. Code is read much more often than it is written, so it should be easy to read it. get_spr_ppr might make sense to you now, but not to other maintainers or even yourself three years from now.
  • Use the correct, standardized Dokeos code header
  • Javascript functions should not be echoed directly. Please store these functions inside htmlHeadExtra variable. Again, see plugin/template.php for this.
  • use $_GET[] and $_POST[]. Assume that register_globals = off. Current Dokeos code has some code that avoid this problem but this code will disappear in the near future - probably in version 1.6.1 or 1.6.2
  • use single quotes for arrays for PHP5 compatability: $foo['bar'] (apparently this was a bug in php5)
  • check your development with error_reporting(E_ALL).
  • Use the correct php tags (start with <?php and end with ?> . Do NOT use short tags <? or ASP-style tags as these rely on php.ini configuration setting: short_open_tag and asp_tags. These settings can cause errors when they are off and Dokeos code assumes that they are set to on.
    The php.ini comments advise not to use short tags or ASP-style tags:
NOTE: Using short tags should be avoided when developing applications or
libraries that are meant for redistribution, or deployment on PHP
servers which are not under your control, because short tags may not
be supported on the target server. For portable, redistributable code,
be sure not to use short tags.
  • use $_SERVER['PHP_SELF'] instead of $PHP_SELF
  • use the $_POST and $_GET array instead of $_REQUEST
  • The fastest way to declare a string is to use single quotation marks because the interpreter does not have to scan het string to perform substitutions. ($variable="this is number $number"). heredoc syntax is even slower because it also has to look for the delimiter.
    $variable='some text here';
    is preferred to
    $variable="some text here";
    the same goes for arrays: use
    $variable['text']='some text here';
    instead of
    $variable["text"]="some text here";
    (see also above)

Code organisation

Code should be neatly organised and commented where necessary.

  • Try to follow a structure similar to that of the plugin/template.php. In fact it's a good idea to just copy the text of that file and paste it into a new file to start programming.
  • Organise code into functions. Any piece of code outside functions is not reusable, and is therefore simply not as good as when it would've been inside a function. If you have only one or two functions you can add them to the file itself. If you need more functions then you should store them a file in the folder of the tool (name them tool.lib.php or toolfunctions.lib.php). Functions that can be reused by multiple tools should be stored in a name.lib.php file inside the inc/lib/ folder.
  • Every function should do only one of the following: get data from database or file system, perform business logic, or do output. Combining these separate functions makes the code difficult to reuse and maintain.
  • In short: Divide and conquer. Every complex problem can be split into several less complex problems. Every ugly code file can be split into separate sections (e.g. functions) that are easier to manage.

There's work to do - general improvements

True or false

  • Change all stored true/false, yes/no, 0/1 database fields and the code that uses these to only one possibility. One example are the admin config settings, which store true or false strings. See the forum [1], also see [2] and [3]

Course creation

There are several places in Dokeos where courses are created, but there is no create_course function in use. Each place calls several functions, juggles with several parameters between them... there's room for improvement.

Use of building blocks

Several parts of Dokeos, notably the new blog, survey, learning path, do not use the Dokeos API provided sortable table or form validator. See also the Developer manual.

Choosing the fastest function

See PHP5 Benchmark and PHP4 Benchmark

There's work to do - specific files

Good example files

These files stand out because they are well-structured, not too long, make good use of the code libraries...

Most scripts using the FormValidator class instead of hardcoded forms

  • admin tools
  • registration and profile
  • ...

Bad example files

If you have some spare time, please lend a hand with these files. They need it.

upgrade script

  • todo: different upgrade script 'files' for upgrading from different platforms
  • todo: use of smaller functions that can be more easly reused, perhaps in shared upgrade library
  • todo: (this has improved) reducing code duplication, smarter way to deal with 100 statements that differ only in one parameter

resource linker

  • very long functions
  • a lot of code duplication
  • massive if/else blocks
  • HTML 4.01 instead of XHTML 1.0
I've already dealt with part of the HTML, but there's still a lot of work here --turboke

export.lib.inc.php

  • functions use a lot of global variables
  • better use of API functions could solve a lot of headaches
  • HTML 4.01 instead of XHTML 1.0

Some work needed

Not too bad, but not too good either. These files should be cleaned up. - this could include the majority of Dokeos scripts, so let's just enter names of files here that you think is worthy of attention...

index.php

very important script. It has been reorganised into functions, and received some css / xhtml attention, but it's just not good enough yet. What we need most is (a) reorganising the functions so that they are neatly split into display | app logic | database functions, and (b) complete xhtml compliance.

Index.php is a big file and it is not well structured. We should make this file a lot cleaner

  • Refactor index.php and do something like this
if ( isset($_uid) )
{
   require 'index_loggedin.inc.php';
}
else
{
    require 'index_anonymous.inc.php';
}
  • Move the actual login code to an .inc file
if ($_POST['submitAuth'])
{
...
  • move the function to an index.inc.php
  • clean the display_digest function and code that uses this function (displays a summary of the agenda and announcements)

Status: as of October 2005, there are people working on the homepage again, with a new view on its structure. For more info, see the Dokeos forum [4].

work/work.php

some cleaning, some code has been moved into functions, but the overall structure is still terrible, however it's easy to clean this file

whoisonline.php

Now the time is hardcoded and set to 30 minutes. This should become an admin-setting. If you want to change it now, you have to change it in claro_init_banner.inc.php and in whoisonline.php

Personal tools