Refactor the 'surveyaction' admin controller in the LimeSurvey Yii port
completed by: Ivan Penchev
mentors: Carsten Schmitz (c_schmitz), Pieter-Jan Speelmans (MrP001), Tom Klingenberg (mot2), Shubham Sachdeva, TMSWhite, Diogo Gonçalves (dionet)
Limesurvey has recently been ported to the Yii framework. The code which has been ported however is not clean code: there are plenty of improvements possible.
The goal of this task is to improve the code so it becomes more readable, consistent and follows the best practices.
Steps:
- Install a subversion client and download the Yii branch from our Subversion respository using a Subversion client.
- Install the Yii branch by pointing the database configuration in /application/config.php to a Limesurvey database. You can also download a test database from http://www.limesurvey.org/exampledata.zip.
- Check out the class which is named in the task title and look for bad code and ways to improve it.
- Improve the code. When it is unclear how to improve it, just contact us using IRC or get in touch directly with one of your mentors. We'll respond as soon as possible.
- Test the changed code.
- Submit a patch created with subversion.
Requirements:
- Good knowledge of PHP
- Experience with a PHP MVC framework
How to improve the code:
- Try splitting up long methods by identifying parts of them with a clear function. Do not forget to give the new methods clear names stating what they do. Clear giveaways of what a block of code does can be comments, but also if-then-else statements.
- Make sure the indentation of the code is correct.
- Rename existing variables and methods so they have clear names.
- Remove duplicate code. When you see similar (or the same) code appear multiple times in the same class, try extracting it into a method.
- Remove HTML from the controller. In the ideal world, no HTML should exist in the controller code. Try moving as much as possible to the view (or create a new view).
- Try to find variables which can be inlined. This can be variables which are never read or read only once.
- Move direct database calls into the model on which the calls are made. These can be spotted by the use of query() and db_exequte_assoc().
Some more possible improvements for this task:
- Split up long methods.
- Move as much html into the view as possible.
- Extract all database calls to the relevant models.
- Remove duplicate code.
- Remove unused variables.
Questions?
Get in touch with us on IRC #limesurvey@freenode.org or leave a comment. Keep in mind that it might take a while before you get a response.