Coding guidelines
From LimeSurvey Manual
NB: This will be replaced by the Code quality guide.
General
This is a free development, everything you do can be useful for others. If the feature is very personal there is a way to turn it into a universal solution. This is the challenge!
Some general rules:
- Be efficient and save resources (memory and runtime). Things might work for a small survey but also try out your new feature with a survey that has 400 question, 5 languages and 20,000 responses (Yes, there are surveys like this out there).
- Keep a fluid dialog with the others coders, they have their own thoughts about what you are doing.
- Ask if you are in doubt.
Documentation
Please document your source code - see this introduction: How to document your source code.
HTML
- As you will notice, a very customized HTML element can take a lot of lines (very expensive if you paid each one (:wink:) ) to avoid that, use 'CSS classes'. In general, use classes to aesthetic related stuff: color, borders, backgrounds, font faces, sizes, etc.
- Respect W3C standards, and try to use the HTML understandable for all browsers. If one of them has a special feature, think if this feature is so special to break down the HTML in other browsers.
- Do not use PHP short tags <? ?>. Always use full PHP tags <?php ?>. Excluded from this is the echo shorthand <?=. Which is allowed as per PSR standard.
- Any rules of the mentioned before: about CSS, well closed HTML tags and JS respect XHTML standards. Give it a try!.
Localization
Moved to https://manual.limesurvey.org/Code_quality_guide#Localization
Bug tracker
- First consider if the reported issue is really a bug. If it is a feature request please move it to the feature tracker.
- If you would like to work on a bug assign it to yourself.
- Try to reproduce the issue.
- If you cannot reproduce ask the user to provide a small example survey to demonstrate the issue. Then set the issue to 'Feedback'. After the user added the necessary information it will automatically be set to 'Assigned' again.
- If the user does not responds for more than a week ask once again for feedback by adding a comment. After a further week without feedback close the issue.
- After you resolved the issue set the status to resolved AND provide the 'Fixed in version' information.
- After you committed a fix and use the proper GIt commit message naming the commit will be automatically assigned to your bug report.
- After a release the release technician will set all issues to 'Closed' that were fixed for this release. That signals to the reporter that a fix is available in the version and it was just released. The release technicial will also add a comment to the close issue: 'Fixed in <Version> <Build number>'
Cross-DB compatibility
LimeSurvey targets several database types it can run on: MySQL, Postgres and Microsoft SQL Server. This demands certain precautions when coding
Parameter binding
Yii/PDO supports parameter binding - use it. Don't inject variables directly into the query or into conditions because this might create a security issue.
Example:
<?php
$oResult = Yii::app()->db
->createCommand()
->select('somefield')
->where("sid = :sid")
->from('{{sometable}}')
->bindParam(":sid", $surveyid, PDO::PARAM_INT);
?>
If you bind the same value several times do NOT use the same parameter name, instead use a different named parameter for each bind:
WRONG:
<?php
$oResult = Yii::app()->db
->createCommand()
->select('somefield')
->where("sid = :sid and parent_sid= :sid ")
->from('{{sometable}}')
->bindParam(":sid", $surveyid, PDO::PARAM_INT);
?>
RIGHT:
<?php
$oResult = Yii::app()->db
->createCommand()
->select('somefield')
->where("sid = :sid1 and parent_sid= :sid2 ")
->from('{{sometable}}')
->bindParam(":sid1", $surveyid, PDO::PARAM_INT)
->bindParam(":sid2", $surveyid, PDO::PARAM_INT);
?>
Quote column name
Some column name must be quoted for DB compatibility. You can use App()->db->quoteColumnName.
Know column needed to be quoted :
- Column name statring with number( column in survey database)
- Reserved
keyword column name
Code complexity
- If using an auxiliary variable makes clear the code or process, just do it. If you are coding a very complex expression, calling to functions with parameters that are other functions:
WRONG:
iAResult= fa(fb(p1,p2),fc(fd(p3),p4),p5)
It will be more readable:
iAuxB = fb(p1,p2)
iAuxD = fd(p3)
iAuxC = fc(iAuxD,p4)
iAResult = fa(iAuxB, iAuxC, p5)
Anyway, if you are in doubt about someone missing the point or you are afraid that your expression is obscure, take the chance of writing it in a more simple way, even if that involves the use of one or more auxiliary variables.
Checklist for new features
If you implement a new feature please make sure that you checked the following things before you create a pull request/commit your new feature:
- Localization: Do all texts (that are usually visible to the user) use the localization/translation system (gT, eT, etc.)?
- Permissions: Does the feature obey the permission system (global permissions, survey permissions)?
- Backward-compatibility: Does the feature still work when updating from any old version? Does it maybe break the update?
- Database compatibility: Does it work on all database types (MySQL, MSSQL, Postgres)?
- Scaling: Does your feature scale? For example; It will work fine with 10 surveys, does it still work performant with 10000?
- Performance: Does your feature use the minimal possible number of SQL queries (see also Scaling).
- QA: Did you test your code for all scenarios? Did you let someone else test or read your code?
- Acceptance tests: Did you write automatic acceptance tests for your specification?
- Discussions: Did you discuss the new feature with the team for feedback?
Also see How_to_contribute_new_features.