MDL-63422 general: review core loop / switch / case / continue
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Sun, 7 Oct 2018 21:25:45 +0000 (23:25 +0200)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Tue, 30 Oct 2018 23:17:59 +0000 (00:17 +0100)
This commit reviews all continue uses in core happening within a
loop / switch / case hierarchy. This does not cover:

- Changes to libraries. Will be handled in another issue / commit.
- Uses out from loops, will be reviewed by other commit.

The policy followed has been:
- When possible, take rid of the continue.
- When clearly the intention was to jump to next element in loop
  change to continue 2
- When it was not clear, keep old behavior switching to break, no
  matter how weird the behavior may be.

18 files changed:
analytics/classes/model.php
blog/classes/privacy/provider.php
competency/classes/privacy/provider.php
grade/querylib.php
lib/gradelib.php
mod/assign/feedback/editpdf/classes/combined_document.php
mod/assign/feedback/editpdf/classes/task/convert_submissions.php
mod/assign/lib.php
mod/chat/chatd.php
mod/choice/lib.php
mod/data/lib.php
mod/feedback/lib.php
mod/forum/lib.php
mod/glossary/lib.php
mod/lesson/lib.php
mod/quiz/lib.php
mod/scorm/lib.php
mod/survey/lib.php

index 199baf4..fa6e91e 100644 (file)
@@ -795,7 +795,7 @@ class model {
                         // skip it and do nothing with it.
                         debugging($this->model->id . ' model predictions processor could not process the sample with id ' .
                             $sampleinfo[0], DEBUG_DEVELOPER);
-                        continue;
+                        continue 2;
                     case 2:
                         // Prediction processors that do not return a prediction score will have the maximum prediction
                         // score.
index e3acab9..588aa39 100644 (file)
@@ -203,7 +203,7 @@ class provider implements
 
                         if (empty($entryids)) {
                             // This should not happen, as the user context should not have been reported then.
-                            continue;
+                            continue 2;
                         }
 
                         list($insql, $inparams) = $DB->get_in_or_equal($entryids, SQL_PARAMS_NAMED);
index 39ff45c..e426ece 100644 (file)
@@ -500,14 +500,13 @@ class provider implements
         foreach ($contextlist as $context) {
             switch ($context->contextlevel) {
                 case CONTEXT_USER:
-                    if ($context->instanceid != $userid) {
+                    if ($context->instanceid == $userid) {
                         // Only delete the user's information when they requested their context to be deleted. We
                         // do not take any action on other user's contexts because we don't own the data there.
-                        continue;
+                        static::delete_user_evidence_of_prior_learning($userid);
+                        static::delete_user_plans($userid);
+                        static::delete_user_competencies($userid);
                     }
-                    static::delete_user_evidence_of_prior_learning($userid);
-                    static::delete_user_plans($userid);
-                    static::delete_user_competencies($userid);
                     break;
 
                 case CONTEXT_COURSE:
index 316a98f..55572a8 100644 (file)
@@ -51,7 +51,7 @@ function grade_get_course_grades($courseid, $userid_or_ids=null) {
 
     switch ($grade_item->gradetype) {
         case GRADE_TYPE_NONE:
-            continue;
+            break;
 
         case GRADE_TYPE_VALUE:
             $item->scaleid = 0;
@@ -178,7 +178,7 @@ function grade_get_course_grade($userid, $courseid_or_ids=null) {
 
         switch ($grade_item->gradetype) {
             case GRADE_TYPE_NONE:
-                continue;
+                break;
 
             case GRADE_TYPE_VALUE:
                 $item->scaleid = 0;
index a9a6829..8960c63 100644 (file)
@@ -472,7 +472,7 @@ function grade_get_grades($courseid, $itemtype, $itemmodule, $iteminstance, $use
 
                 switch ($grade_item->gradetype) {
                     case GRADE_TYPE_NONE:
-                        continue;
+                        break;
 
                     case GRADE_TYPE_VALUE:
                         $item->scaleid = 0;
index 91fc23b..08f6163 100644 (file)
@@ -187,7 +187,7 @@ class combined_document {
                 $status = $file->get('status');
                 switch ($status) {
                     case \core_files\conversion::STATUS_COMPLETE:
-                        continue;
+                        continue 2;
                         break;
                     default:
                         $converter->poll_conversion($conversion);
index 74a781c..68295fd 100644 (file)
@@ -108,7 +108,7 @@ class convert_submissions extends scheduled_task {
                         case combined_document::STATUS_READY:
                         case combined_document::STATUS_PENDING_INPUT:
                             // The document has not been converted yet or is somehow still ready.
-                            continue;
+                            continue 2;
                     }
                     document_services::get_page_images_for_attempt(
                             $assignment,
index edc5c50..775374b 100644 (file)
@@ -534,10 +534,9 @@ function mod_assign_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionsubmit':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionsubmit', 'assign');
                 }
-                $descriptions[] = get_string('completionsubmit', 'assign');
                 break;
             default:
                 break;
index 1cfee64..47a4297 100644 (file)
@@ -1007,7 +1007,7 @@ while (true) {
                         if (!preg_match('/beep=([^&]*)[& ]/', $data, $info)) {
                             $daemon->trace('Beep sidekick did not contain a valid userid', E_USER_WARNING);
                             $daemon->dismiss_ufo($handle, true, 'Request with malformed data; connection closed');
-                            continue;
+                            continue 2;
                         } else {
                             $customdata = array('beep' => intval($info[1]));
                         }
@@ -1017,7 +1017,7 @@ while (true) {
                         if (!preg_match('/chat_message=([^&]*)[& ]chat_msgidnr=([^&]*)[& ]/', $data, $info)) {
                             $daemon->trace('Message sidekick did not contain a valid message', E_USER_WARNING);
                             $daemon->dismiss_ufo($handle, true, 'Request with malformed data; connection closed');
-                            continue;
+                            continue 2;
                         } else {
                             $customdata = array('message' => $info[1], 'index' => $info[2]);
                         }
@@ -1025,7 +1025,7 @@ while (true) {
                     default:
                         $daemon->trace('UFO with '.$handle.': Request with unknown type; connection closed', E_USER_WARNING);
                         $daemon->dismiss_ufo($handle, true, 'Request with unknown type; connection closed');
-                        continue;
+                        continue 2;
                     break;
                 }
 
index 3256a8c..f15788e 100644 (file)
@@ -1453,10 +1453,9 @@ function mod_choice_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionsubmit':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionsubmit', 'choice');
                 }
-                $descriptions[] = get_string('completionsubmit', 'choice');
                 break;
             default:
                 break;
index 7bb1310..3bebba3 100644 (file)
@@ -4555,10 +4555,9 @@ function mod_data_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionentries':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionentriesdesc', 'data', $val);
                 }
-                $descriptions[] = get_string('completionentriesdesc', 'data', $val);
                 break;
             default:
                 break;
index 191602a..bde69f4 100644 (file)
@@ -3097,10 +3097,9 @@ function mod_feedback_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionsubmit':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionsubmit', 'feedback');
                 }
-                $descriptions[] = get_string('completionsubmit', 'feedback');
                 break;
             default:
                 break;
index b11b3b4..e94a720 100644 (file)
@@ -8563,22 +8563,19 @@ function mod_forum_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completiondiscussions':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completiondiscussionsdesc', 'forum', $val);
                 }
-                $descriptions[] = get_string('completiondiscussionsdesc', 'forum', $val);
                 break;
             case 'completionreplies':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionrepliesdesc', 'forum', $val);
                 }
-                $descriptions[] = get_string('completionrepliesdesc', 'forum', $val);
                 break;
             case 'completionposts':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionpostsdesc', 'forum', $val);
                 }
-                $descriptions[] = get_string('completionpostsdesc', 'forum', $val);
                 break;
             default:
                 break;
index 53e2ba4..a2bf130 100644 (file)
@@ -4314,10 +4314,9 @@ function mod_glossary_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionentries':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionentriesdesc', 'glossary', $val);
                 }
-                $descriptions[] = get_string('completionentriesdesc', 'glossary', $val);
                 break;
             default:
                 break;
index 2963827..9e4d91e 100644 (file)
@@ -1745,16 +1745,14 @@ function mod_lesson_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionendreached':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionendreached_desc', 'lesson', $val);
                 }
-                $descriptions[] = get_string('completionendreached_desc', 'lesson', $val);
                 break;
             case 'completiontimespent':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completiontimespentdesc', 'lesson', format_time($val));
                 }
-                $descriptions[] = get_string('completiontimespentdesc', 'lesson', format_time($val));
                 break;
             default:
                 break;
index 9fd12e9..8f698ee 100644 (file)
@@ -2266,16 +2266,14 @@ function mod_quiz_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionattemptsexhausted':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionattemptsexhausteddesc', 'quiz');
                 }
-                $descriptions[] = get_string('completionattemptsexhausteddesc', 'quiz');
                 break;
             case 'completionpass':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionpassdesc', 'quiz', format_time($val));
                 }
-                $descriptions[] = get_string('completionpassdesc', 'quiz', format_time($val));
                 break;
             default:
                 break;
index ee88c55..6b74c23 100644 (file)
@@ -1778,30 +1778,27 @@ function mod_scorm_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionstatusrequired':
-                if (is_null($val)) {
-                    continue;
-                }
-                // Determine the selected statuses using a bitwise operation.
-                $cvalues = array();
-                foreach (scorm_status_options(true) as $bit => $string) {
-                    if (($val & $bit) == $bit) {
-                        $cvalues[] = $string;
+                if (!is_null($val)) {
+                    // Determine the selected statuses using a bitwise operation.
+                    $cvalues = array();
+                    foreach (scorm_status_options(true) as $bit => $string) {
+                        if (($val & $bit) == $bit) {
+                            $cvalues[] = $string;
+                        }
                     }
+                    $statusstring = implode(', ', $cvalues);
+                    $descriptions[] = get_string('completionstatusrequireddesc', 'scorm', $statusstring);
                 }
-                $statusstring = implode(', ', $cvalues);
-                $descriptions[] = get_string('completionstatusrequireddesc', 'scorm', $statusstring);
                 break;
             case 'completionscorerequired':
-                if (is_null($val)) {
-                    continue;
+                if (!is_null($val)) {
+                    $descriptions[] = get_string('completionscorerequireddesc', 'scorm', $val);
                 }
-                $descriptions[] = get_string('completionscorerequireddesc', 'scorm', $val);
                 break;
             case 'completionstatusallscos':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionstatusallscos', 'scorm');
                 }
-                $descriptions[] = get_string('completionstatusallscos', 'scorm');
                 break;
             default:
                 break;
@@ -1922,4 +1919,4 @@ function mod_scorm_core_calendar_get_valid_event_timestart_range(\calendar_event
     }
 
     return [$mindate, $maxdate];
-}
\ No newline at end of file
+}
index 1f630ec..73e0d78 100644 (file)
@@ -1201,10 +1201,9 @@ function mod_survey_get_completion_active_rule_descriptions($cm) {
     foreach ($cm->customdata['customcompletionrules'] as $key => $val) {
         switch ($key) {
             case 'completionsubmit':
-                if (empty($val)) {
-                    continue;
+                if (!empty($val)) {
+                    $descriptions[] = get_string('completionsubmit', 'survey');
                 }
-                $descriptions[] = get_string('completionsubmit', 'survey');
                 break;
             default:
                 break;