From bd5fdcfccd3c5953259fb605b047a91761758260 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 7 Oct 2018 23:25:45 +0200 Subject: [PATCH] MDL-63422 general: review core loop / switch / case / continue 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. --- analytics/classes/model.php | 2 +- blog/classes/privacy/provider.php | 2 +- competency/classes/privacy/provider.php | 9 +++--- grade/querylib.php | 4 +-- lib/gradelib.php | 2 +- .../editpdf/classes/combined_document.php | 2 +- .../classes/task/convert_submissions.php | 2 +- mod/assign/lib.php | 5 ++- mod/chat/chatd.php | 6 ++-- mod/choice/lib.php | 5 ++- mod/data/lib.php | 5 ++- mod/feedback/lib.php | 5 ++- mod/forum/lib.php | 15 ++++----- mod/glossary/lib.php | 5 ++- mod/lesson/lib.php | 10 +++--- mod/quiz/lib.php | 10 +++--- mod/scorm/lib.php | 31 +++++++++---------- mod/survey/lib.php | 5 ++- 18 files changed, 54 insertions(+), 71 deletions(-) diff --git a/analytics/classes/model.php b/analytics/classes/model.php index 199baf4c1c0d9..fa6e91eae883a 100644 --- a/analytics/classes/model.php +++ b/analytics/classes/model.php @@ -795,7 +795,7 @@ private function format_predictor_predictions($predictorresult) { // 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. diff --git a/blog/classes/privacy/provider.php b/blog/classes/privacy/provider.php index e3acab93d49cb..588aa392458b6 100644 --- a/blog/classes/privacy/provider.php +++ b/blog/classes/privacy/provider.php @@ -203,7 +203,7 @@ public static function export_user_data(approved_contextlist $contextlist) { 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); diff --git a/competency/classes/privacy/provider.php b/competency/classes/privacy/provider.php index 39ff45c5048d7..e426eced497fb 100644 --- a/competency/classes/privacy/provider.php +++ b/competency/classes/privacy/provider.php @@ -500,14 +500,13 @@ public static function delete_data_for_user(approved_contextlist $contextlist) { 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: diff --git a/grade/querylib.php b/grade/querylib.php index 316a98ff64e97..55572a8cece6d 100644 --- a/grade/querylib.php +++ b/grade/querylib.php @@ -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; diff --git a/lib/gradelib.php b/lib/gradelib.php index a9a6829eaae15..8960c63c2306a 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -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; diff --git a/mod/assign/feedback/editpdf/classes/combined_document.php b/mod/assign/feedback/editpdf/classes/combined_document.php index 91fc23be6ad07..08f6163f4e708 100644 --- a/mod/assign/feedback/editpdf/classes/combined_document.php +++ b/mod/assign/feedback/editpdf/classes/combined_document.php @@ -187,7 +187,7 @@ public function refresh_files() { $status = $file->get('status'); switch ($status) { case \core_files\conversion::STATUS_COMPLETE: - continue; + continue 2; break; default: $converter->poll_conversion($conversion); diff --git a/mod/assign/feedback/editpdf/classes/task/convert_submissions.php b/mod/assign/feedback/editpdf/classes/task/convert_submissions.php index 74a781c71742a..68295fd840ea3 100644 --- a/mod/assign/feedback/editpdf/classes/task/convert_submissions.php +++ b/mod/assign/feedback/editpdf/classes/task/convert_submissions.php @@ -108,7 +108,7 @@ public function execute() { 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, diff --git a/mod/assign/lib.php b/mod/assign/lib.php index edc5c50a32306..775374b589cc1 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -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; diff --git a/mod/chat/chatd.php b/mod/chat/chatd.php index 1cfee64ec378f..47a4297d90a60 100644 --- a/mod/chat/chatd.php +++ b/mod/chat/chatd.php @@ -1007,7 +1007,7 @@ public function cli_switch($switch, $param = null) { 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 @@ public function cli_switch($switch, $param = null) { 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 @@ public function cli_switch($switch, $param = null) { 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; } diff --git a/mod/choice/lib.php b/mod/choice/lib.php index 3256a8c1f978d..f15788e0b1fdd 100644 --- a/mod/choice/lib.php +++ b/mod/choice/lib.php @@ -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; diff --git a/mod/data/lib.php b/mod/data/lib.php index 7bb1310ee0419..3bebba3cabc09 100644 --- a/mod/data/lib.php +++ b/mod/data/lib.php @@ -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; diff --git a/mod/feedback/lib.php b/mod/feedback/lib.php index 191602ad5375f..bde69f4a94f32 100644 --- a/mod/feedback/lib.php +++ b/mod/feedback/lib.php @@ -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; diff --git a/mod/forum/lib.php b/mod/forum/lib.php index b11b3b46263e0..e94a720b43b19 100644 --- a/mod/forum/lib.php +++ b/mod/forum/lib.php @@ -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; diff --git a/mod/glossary/lib.php b/mod/glossary/lib.php index 53e2ba4620a19..a2bf1305ff212 100644 --- a/mod/glossary/lib.php +++ b/mod/glossary/lib.php @@ -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; diff --git a/mod/lesson/lib.php b/mod/lesson/lib.php index 2963827e642be..9e4d91e5500f2 100644 --- a/mod/lesson/lib.php +++ b/mod/lesson/lib.php @@ -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; diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 9fd12e97727a1..8f698ee71675e 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -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; diff --git a/mod/scorm/lib.php b/mod/scorm/lib.php index ee88c55e98277..6b74c23e3bf8c 100644 --- a/mod/scorm/lib.php +++ b/mod/scorm/lib.php @@ -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 +} diff --git a/mod/survey/lib.php b/mod/survey/lib.php index 1f630ecabf976..73e0d78a1035a 100644 --- a/mod/survey/lib.php +++ b/mod/survey/lib.php @@ -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;