patternphpMinor

Manipulating survey questions

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

questionssurveymanipulating

Problem

I've been a developer for many years, but have always put together files of lists of functions to do everything that needed to be done (instead of using classes/objects like I should be). I've recently started forcing myself to try to get into more object-oriented programming.

I wanted to get some feedback on whether or not I'm implementing classes/objects 'properly' and 'effectively'.

I'm manipulating "Questions" within "Surveys". "Questions" also have sub-parts such as "Choices", but all-in-all my classes are set up in the same format.

I have a Questions.class.php (which contains all items related to getting/setting/updating/deleting questions) and a QuestionList.class.php (which grabs lists of questions from the database).

```
_table = 'TABLE NAME OF QUESTIONS THEMSELVES';
$this->_surveys_table = 'TABLE NAME OF SURVEYS (A SURVEY CONTAINS QUESTIONS)';

//When I create an object in external code, I pass it either an ID of a question that exists in my database, or NULL to set up a blank question that doesn't yet exist in the database.
if(isset($question_id) && is_int($question_id)) {
$this->_question = $this->get($question_id);
} else {
$this->_question = array();
}

//This is just a file of error messages--stole the concept from a large Authentication class
require "language.php";
$this->lang = $lang;
}

//Literally just gets the question from the database and returns an error if it doesn't exist.
public function get($question_id) {
$return['error'] = true;

global $DBH;
$query = "SELECT * FROM {$this->_table} WHERE question_id = :question_id";
$q = $DBH->prepare($query);
$q->bindValue(':question_id', $question_id, \PDO::PARAM_INT);
$q->execute();

if(!$row = $q->fetch(\PDO::FETCH_ASSOC)) {
$return['message'] = $this->lang['questions_nomatch'];
return $return;
}

ret

Solution

DO NOT USE GLOBALS

Message handling is not models responsibility.

Functions should do as few things as possible, preferably one.

-
Try not to reinvent the wheel. What you are trying to accomplish here is easily done using ORM (ex: Doctrine) or Active Record.

-
Caller should provide function with valid arguments (expect either array or int/string). Null is rarely a valid argument. Definitely not in your constructor.

Below code can be improved but I didn't want to refactor it further as scale of project is very low and it might have added unnecessary complexity. I have used PDO wrapper from here.

bindValue(':question_id', $id, \PDO::PARAM_INT);
        if(!$stmt->fetch(\PDO::FETCH_ASSOC)) {
            return null;
        }
        $model = new self;
        $model->setAttributes($stmt->fetch(\PDO::FETCH_ASSOC));
        return $model;
    }

    public static function findAllBySurveyId($surveyId) {
        /** @var PDOStatement $stmt */
        $stmt = DB::prepare("SELECT * FROM ".self::table()." WHERE survey_id = :survey_id ORDER BY question_order");
        $stmt->bindValue(':survey_id', $surveyId, \PDO::PARAM_INT);
        return self::getAllModels($stmt);
    }

    public static function findAll() {
        /** @var PDOStatement $stmt */
        $stmt = DB::prepare("SELECT * FROM ".self::table());
        return self::getAllModels($stmt);
    }

    public function add() {
        /** @var PDOStatement $stmt */
        $this->question_order = $this->getOrder();
        $stmt = DB::prepare(/*my database insert query is here.*/);
        if(!$stmt->execute()) {
            return false;
        }
        $this->setAttributes(['question_id' => DB::lastInsertId()]);
        $this->incrementOrder();
        return true;
    }

    public function update() {
        /** @var PDOStatement $stmt */
        $stmt = DB::prepare(/*my database update query is here.*/);
        return $stmt->execute();
    }

    public function delete() {
        $stmt = DB::prepare("DELETE FROM {$this->table()} WHERE question_id = :question_id");
        return $stmt->execute([':question_id' => $this->question_id]);
    }

    public function setAttributes(array $attributes) {
        foreach($attributes as $attribute => $value) {
            $this->$attribute = $value;
        }
    }

    private function getOrder() {
        return 1;
    }

    private function incrementOrder() {
        $this->question_order = $this->getOrder() + 1;
    }

    private function getAllModels(PDOStatement $stmt) {
        $models = [];
        while($row = $stmt->fetch(\PDO::FETCH_ASSOC)) {
            $model = new self;
            $model->setAttributes($row);
            $models[] = $model;
        }
        return $models;
    }
}

$question1 = new Question();
$question1->setAttributes([
    'question' => $question,
    'question_multi' => $question_multi,
    'survey_id' => $survey_id,
]);

if($question1->add()) {
    // successful
}

$question2 = (new Question())->findById(1);
$question2->survey_id = $survey_id;
$question2->question = $question;
$question2->question_multi = $question_multi;

if($question2->update()) {
    // successful
}

Code Snippets

<?php

class Question {

    // attributes
    public $question;
    public $question_id; // used snake case as names should be the same as column names in db.
    public $question_multi;
    public $survey_id;
    public $question_order;

    public function table() {
        return 'TABLE NAME OF QUESTIONS THEMSELVES';
    }

    public static function findById($id) {
        /** @var PDOStatement $stmt */
        $stmt = DB::prepare("SELECT * FROM ".self::table()." WHERE question_id = :question_id");
        $stmt->bindValue(':question_id', $id, \PDO::PARAM_INT);
        if(!$stmt->fetch(\PDO::FETCH_ASSOC)) {
            return null;
        }
        $model = new self;
        $model->setAttributes($stmt->fetch(\PDO::FETCH_ASSOC));
        return $model;
    }

    public static function findAllBySurveyId($surveyId) {
        /** @var PDOStatement $stmt */
        $stmt = DB::prepare("SELECT * FROM ".self::table()." WHERE survey_id = :survey_id ORDER BY question_order");
        $stmt->bindValue(':survey_id', $surveyId, \PDO::PARAM_INT);
        return self::getAllModels($stmt);
    }

    public static function findAll() {
        /** @var PDOStatement $stmt */
        $stmt = DB::prepare("SELECT * FROM ".self::table());
        return self::getAllModels($stmt);
    }

    public function add() {
        /** @var PDOStatement $stmt */
        $this->question_order = $this->getOrder();
        $stmt = DB::prepare(/*my database insert query is here.*/);
        if(!$stmt->execute()) {
            return false;
        }
        $this->setAttributes(['question_id' => DB::lastInsertId()]);
        $this->incrementOrder();
        return true;
    }

    public function update() {
        /** @var PDOStatement $stmt */
        $stmt = DB::prepare(/*my database update query is here.*/);
        return $stmt->execute();
    }

    public function delete() {
        $stmt = DB::prepare("DELETE FROM {$this->table()} WHERE question_id = :question_id");
        return $stmt->execute([':question_id' => $this->question_id]);
    }

    public function setAttributes(array $attributes) {
        foreach($attributes as $attribute => $value) {
            $this->$attribute = $value;
        }
    }

    private function getOrder() {
        return 1;
    }

    private function incrementOrder() {
        $this->question_order = $this->getOrder() + 1;
    }

    private function getAllModels(PDOStatement $stmt) {
        $models = [];
        while($row = $stmt->fetch(\PDO::FETCH_ASSOC)) {
            $model = new self;
            $model->setAttributes($row);
            $models[] = $model;
        }
        return $models;
    }
}

$question1 = new Question();
$question1->setAttributes([
    'question' => $question,
    'question_multi' => $question_multi,
    'survey_id' => $survey_id,
]);

if($question1->add()) {
    // successful
}

$question2 = (new Question())->findById(1);
$question2->survey_id = $survey_id;
$question2->question = $question;
$question

Context

StackExchange Code Review Q#141077, answer score: 3

Revisions (0)

No revisions yet.