Actions

Coding guidelines

From LimeSurvey Manual

Revision as of 13:31, 3 August 2021 by Ollehar (talk | contribs) (→‎Naming convention)
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)

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!

"The difficulty within a solution is to solve it in a simple way" (Lance Burton, magician)

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

  1. First consider if the reported issue is really a bug. If it is a feature request please move it to the feature tracker.
  2. If you would like to work on a bug assign it to yourself.
  3. Try to reproduce the issue.
    1. 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.
    2. 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.
  4. After you resolved the issue set the status to resolved AND provide the 'Fixed in version' information.
  5. After you committed a fix and use the proper GIt commit message naming the commit will be automatically assigned to your bug report.
  6. 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 key word 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.