HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Flex app for quizzing

Submitted by: @import:stackexchange-codereview··
0
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

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:

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.