Skip to main content
Code Review

Return to Answer

deleted 525 characters in body
Source Link

Instead of setting the name attribute uniquely on each button

<button id="beginner" name="beginner" value="beginner">Beginner</button><br><br>

Set the name on all three the same - e.g. name="difficulty". That can make the query logic much simpler:

function getDifficulty() {
 foreach (['advanced', 'intermediate', 'beginner'] as $difficultyLevel) {
 //initialize to avoid undefined variables if (isset($_POST[$difficultyLevel])) {
 return $_POST[$difficultyLevel];
 }
  POST }
}
//initializenot set
$exercise_selected = $difficulty = null;
if ($difficultyLevel = getDifficultyisset()$_POST['difficulty']) {
 $get_exercises = $con->prepare("SELECT exercise_name, difficulty FROM exercises WHERE difficulty = ?");
 $get_exercises->bind_param("s", difficultyLevel$_POST['difficulty']);
 $get_exercises->execute();
 $get_exercises->store_result();
 $num_of_rows = $get_exercises->num_rows;
 $get_exercises->bind_result($exercise_selected, $difficulty);
}

While the HTML form might only lead to one difficulty level having a value that is truethy with in $_POST, there isn't anything that prevents the user from submitting the form another way (e.g. manually in browser, a POST tool like Postman , etc.) If that did happen then that would lead to the last one over-writing any previous values for the output variables, which is also possible with the original code. If only one difficulty level should have a truethy value in $_POST then the logic could be updated.

function getDifficulty() {
 foreach (['advanced', 'intermediate', 'beginner'] as $difficultyLevel) {
  if (isset($_POST[$difficultyLevel])) {
 return $_POST[$difficultyLevel];
 }
  }
}
//initialize 
$exercise_selected = $difficulty = null;
if ($difficultyLevel = getDifficulty()) {
 $get_exercises = $con->prepare("SELECT exercise_name, difficulty FROM exercises WHERE difficulty = ?");
 $get_exercises->bind_param("s", difficultyLevel);
 $get_exercises->execute();
 $get_exercises->store_result();
 $num_of_rows = $get_exercises->num_rows;
 $get_exercises->bind_result($exercise_selected, $difficulty);
}

While the HTML form might only lead to one difficulty level having a value that is truethy with in $_POST, there isn't anything that prevents the user from submitting the form another way (e.g. manually in browser, a POST tool like Postman , etc.) If that did happen then that would lead to the last one over-writing any previous values for the output variables, which is also possible with the original code. If only one difficulty level should have a truethy value in $_POST then the logic could be updated.

Instead of setting the name attribute uniquely on each button

<button id="beginner" name="beginner" value="beginner">Beginner</button><br><br>

Set the name on all three the same - e.g. name="difficulty". That can make the query logic much simpler:

//initialize to avoid undefined variables if POST not set
$exercise_selected = $difficulty = null;
if (isset($_POST['difficulty']) {
 $get_exercises = $con->prepare("SELECT exercise_name, difficulty FROM exercises WHERE difficulty = ?");
 $get_exercises->bind_param("s", $_POST['difficulty']);
 $get_exercises->execute();
 $get_exercises->store_result();
 $num_of_rows = $get_exercises->num_rows;
 $get_exercises->bind_result($exercise_selected, $difficulty);
}
Take query out of loop
Source Link

The mixing of PHP within HTML is not great. It is recommended to have the PHP fetch data and then within the HTML just output any data that might have been retrieved.

There are quite a few duplicate lines in these three blocks:

There are quite a few duplicate lines in these three blocks:

The mixing of PHP within HTML is not great. It is recommended to have the PHP fetch data and then within the HTML just output any data that might have been retrieved.

There are quite a few duplicate lines in these three blocks:

Take query out of loop
Source Link
//this array could befunction declaredgetDifficulty() as{
 a constant somewhere
$difficultyLevels =foreach ['beginner'(['advanced', 'intermediate', 'advanced'];
foreach ($difficultyLevels'beginner'] as $difficultyLevel) {
 if (!isset($_POST[$difficultyLevel])) {
 continue; return $_POST[$difficultyLevel];
 }
 }
}
//initialize 
$exercise_selected = $difficulty = null;
if ($difficultyLevel = getDifficulty()) {
 $get_exercises = $con->prepare("SELECT exercise_name, difficulty FROM exercises WHERE difficulty = ?");
 $get_exercises->bind_param("s", $_POST[difficultyLevel']difficultyLevel);
 $get_exercises->execute();
 $get_exercises->store_result();
 $num_of_rows = $get_exercises->num_rows;
 $get_exercises->bind_result($exercise_selected, $difficulty);
}

While the HTML form might only lead to one difficulty level having a value that is truethy with in $_POST, there isn't anything that prevents the user from submitting the form another way (e.g. manually in browser, a POST tool like Postman, etc.) If that did happen then that would lead to the last one over-writing any previous values for the output variables, which is also possible with the original code. If only one difficulty level should have a truethy value in $_POST then the logic could be updated - e.g. add a break from the loop once a query is executed.

//this array could be declared as a constant somewhere
$difficultyLevels = ['beginner', 'intermediate', 'advanced'];
foreach ($difficultyLevels as $difficultyLevel) {
 if (!isset($_POST[$difficultyLevel])) {
 continue;
 }
 $get_exercises = $con->prepare("SELECT exercise_name, difficulty FROM exercises WHERE difficulty = ?");
 $get_exercises->bind_param("s", $_POST[difficultyLevel']);
 $get_exercises->execute();
 $get_exercises->store_result();
 $num_of_rows = $get_exercises->num_rows;
 $get_exercises->bind_result($exercise_selected, $difficulty);
}

While the HTML form might only lead to one difficulty level having a value that is truethy with in $_POST, there isn't anything that prevents the user from submitting the form another way (e.g. manually in browser, a POST tool like Postman, etc.) If that did happen then that would lead to the last one over-writing any previous values for the output variables, which is also possible with the original code. If only one difficulty level should have a truethy value in $_POST then the logic could be updated - e.g. add a break from the loop once a query is executed.

function getDifficulty() {
 foreach (['advanced', 'intermediate', 'beginner'] as $difficultyLevel) {
 if (isset($_POST[$difficultyLevel])) {
  return $_POST[$difficultyLevel];
 }
 }
}
//initialize 
$exercise_selected = $difficulty = null;
if ($difficultyLevel = getDifficulty()) {
 $get_exercises = $con->prepare("SELECT exercise_name, difficulty FROM exercises WHERE difficulty = ?");
 $get_exercises->bind_param("s", difficultyLevel);
 $get_exercises->execute();
 $get_exercises->store_result();
 $num_of_rows = $get_exercises->num_rows;
 $get_exercises->bind_result($exercise_selected, $difficulty);
}

While the HTML form might only lead to one difficulty level having a value that is truethy with in $_POST, there isn't anything that prevents the user from submitting the form another way (e.g. manually in browser, a POST tool like Postman, etc.) If that did happen then that would lead to the last one over-writing any previous values for the output variables, which is also possible with the original code. If only one difficulty level should have a truethy value in $_POST then the logic could be updated.

added 85 characters in body
Source Link
Loading
added 236 characters in body
Source Link
Loading
added 151 characters in body
Source Link
Loading
Source Link
Loading
default

AltStyle によって変換されたページ (->オリジナル) /