patternphpMinor
Flex app for quizzing
Viewed 0 times
flexforquizzingapp
Problem
This is from a flex app that is for quizzing. It calls the database for the question and then checks to see if it has been tagged (bookmarked) by a user already. "Question" is a value object and "Tag" is also a value object.
I just wanted to post it to see how it could be better. It works fine, but I feel that since I am new to this, it probably isn't best.
```
public function getQuestionByNumber($itemNum, $userID) {
$stmt = mysqli_prepare($this->connection,
"SELECT
questions.id, questions.question_number, questions.chapter_id, questions.section_id,
questions.question_txt, questions.answers, questions.feedbacks, questions.correct_answer,
questions.question_type, questions.module, questions.shuffle_answer,
questions.tags
FROM questions where questions.question_number=?");
$this->throwExceptionOnError();
mysqli_stmt_bind_param($stmt, 'i', $itemNum); $this->throwExceptionOnError();
mysqli_stmt_execute($stmt); $this->throwExceptionOnError();
$row = new Question();
mysqli_stmt_bind_result($stmt, $row->id, $row->question_number, $row->chapter_id, $row->section_id,
$row->question_txt, $row->answers, $row->feedbacks, $row->correct_answer,
$row->question_type, $row->module, $row->shuffle_answer, $row->tags);
if (mysqli_stmt_fetch($stmt)) {
} else {
return null;
}
mysqli_stmt_free_result($stmt);
mysqli_stmt_close($stmt);
$stmt2 = mysqli_prepare($this->connection,
"SELECT
id
FROM tagged where user_id =? && question_number=?");
$this->throwExceptionOnError();
mysqli_stmt_bind_param($stmt2, 'ii', $userID, $itemNum);
$this->throwExceptionOnError();
mysqli_stmt_execute($st
I just wanted to post it to see how it could be better. It works fine, but I feel that since I am new to this, it probably isn't best.
```
public function getQuestionByNumber($itemNum, $userID) {
$stmt = mysqli_prepare($this->connection,
"SELECT
questions.id, questions.question_number, questions.chapter_id, questions.section_id,
questions.question_txt, questions.answers, questions.feedbacks, questions.correct_answer,
questions.question_type, questions.module, questions.shuffle_answer,
questions.tags
FROM questions where questions.question_number=?");
$this->throwExceptionOnError();
mysqli_stmt_bind_param($stmt, 'i', $itemNum); $this->throwExceptionOnError();
mysqli_stmt_execute($stmt); $this->throwExceptionOnError();
$row = new Question();
mysqli_stmt_bind_result($stmt, $row->id, $row->question_number, $row->chapter_id, $row->section_id,
$row->question_txt, $row->answers, $row->feedbacks, $row->correct_answer,
$row->question_type, $row->module, $row->shuffle_answer, $row->tags);
if (mysqli_stmt_fetch($stmt)) {
} else {
return null;
}
mysqli_stmt_free_result($stmt);
mysqli_stmt_close($stmt);
$stmt2 = mysqli_prepare($this->connection,
"SELECT
id
FROM tagged where user_id =? && question_number=?");
$this->throwExceptionOnError();
mysqli_stmt_bind_param($stmt2, 'ii', $userID, $itemNum);
$this->throwExceptionOnError();
mysqli_stmt_execute($st
Solution
You could combine your two SQL queries so you don't have to hit the database twice.
The new SQL statement would be something like this:
The COALESCE function would return the id from the tagged table if one exists, otherwise it returns 0. This eliminates the need for your final if/else statement and gives you a direct mapping for $row->tagged.
You should also release all result sets and close all connections before issuing a return statement. Any code after a return statement will not be executed.
The new SQL statement would be something like this:
SELECT
q.id, q.question_number, q.chapter_id, q.section_id,
q.question_txt, q.answers, q.feedbacks, q.correct_answer,
q.question_type, q.module, q.shuffle_answer,
q.tags, COALESCE(t.id, 0) As tagged
FROM questions q
LEFT JOIN tagged t ON q.question_number = t.question_number AND t.user_id = ?
WHERE q.question_number=?The COALESCE function would return the id from the tagged table if one exists, otherwise it returns 0. This eliminates the need for your final if/else statement and gives you a direct mapping for $row->tagged.
You should also release all result sets and close all connections before issuing a return statement. Any code after a return statement will not be executed.
Code Snippets
SELECT
q.id, q.question_number, q.chapter_id, q.section_id,
q.question_txt, q.answers, q.feedbacks, q.correct_answer,
q.question_type, q.module, q.shuffle_answer,
q.tags, COALESCE(t.id, 0) As tagged
FROM questions q
LEFT JOIN tagged t ON q.question_number = t.question_number AND t.user_id = ?
WHERE q.question_number=?Context
StackExchange Code Review Q#4304, answer score: 3
Revisions (0)
No revisions yet.