Details

    • Type: Sub-task
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Component/s: Coding style
    • Labels:
      None

      Description

      The current style guide says that you should shorten control structures (if) by doing the various conditional checks ahead of time. The problem is this stops short-circuiting which can both be a major boon for performance, and can be used for explicit behavior control, which we actually do in almost every file:

      defined('MOODLE_INTERNAL') || die();
      

      This relies on short-circuiting to work at all.

      The example giving in the style guide if even an example of a unneeded performance hit:

      $coursecategory = ($element['object']->is_course_item() or $element['object']->is_category_item());
      $scalevalue = in_array($element['object']->gradetype, array(GRADE_TYPE_SCALE, GRADE_TYPE_VALUE));
       
      if ($coursecategory and $scalevalue) {
      

      The in_array() is now done every time, even if the first part of the check fails and it will never be used.

      We should have a policy that allows the wrapping of large control structures, and give guidance about things like where you place the operators (start of end of line), parentheses, etc.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated: