OOP PHP for handling upvoting

kinda new to OOP, and I wanna know if i'm heading into the right direction. Also I should mention I'm not using AJAX

AnswerVoteManager.php

```
answerID;
}

/**
* @param mixed $answerID
*/
public function setAnswerID($answerID)
{
$this->answerID = $answerID;
}

/**
* @return mixed
*/
public function getTitle()
{
return $this->title;
}

/**
* @param mixed $title
*/
public function setTitle($title)
{
$this->title = $title;
}

/**
* @return mixed
*/
public function getVoter()
{
return $this->voter;
}

/**
* @param mixed $voter
*/
public function setVoter($voter)
{
$this->voter = $voter;
}

/**
* @return mixed
*/
public function getVoteType()
{
return $this->vote_type;
}

/**
* @param mixed $vote_type
*/
public function setVoteType($vote_type)
{
$this->vote_type = $vote_type;
}
public function __construct($dbh)
{
$this->dbh = $dbh;

}
public function getCurrentvote($answerid,$username)
{
$sql = "SELECT vote_type from answer_votes WHERE answerid=:answerid and username=:username";
$stmt = $this->dbh->prepare($sql);
$stmt->bindValue(':answerid',$answerid);
$stmt->bindValue(':username',$username);
$stmt->execute();
$type = (string) $stmt->fetchColumn();
if(empty($type))
{
return null;
} else {
return (string) $type;
}
}
private function voted()
{
$sql = "SELECT vote_type from answer_votes where answerID=:answerid and username=:username";
$stmt = $this->dbh->prepare($sql);
$stmt->bindValue(':answerid',$this->answerID);
$stmt->bindValue(':username',$this->voter);
$stmt->execute();
$exists = $stmt->rowCount();
if($exists)
{
return true;
} else {
return false;
}
}
private function redirect()
{
header("Location:" . $_SERVER['REQUEST_URI']);
exit();
}
private function updateVote()
{
$sql = "UPDATE answer_votes set vote_type=:vote_type";
$stmt = $this->dbh->prepare($sql);
$stmt->bindValue(':vote_type',$this->getVoteType(

I think you might want to reconsider your overall interface and class design here. While I appreciate that you are trying to work with interfaces, I don't think that the interface may be the best way to do what you are wanting to do here. You should always think about the real-world aspects of what you are trying to model. Here, your question and answer classes are not really represent questions and answers at all. Rather, they seemingly are really just two different types of votes you have in your application.

Why is this important?

Because if you just have two different types of the same object, then you might think towards inheritance as part of your solution as opposed to interfaces which are typically used to describe two objects of different types that exhibit similar behavior.

Do you notice that your implementations of the common interface methods across the two classes are almost exactly the same (with DB table you are acting against being the main difference)? This should be a red flag that perhaps this should refactored such that you don't need to re-implement all this functionality in every class that needs voting capability.

There are a couple of ways you might approach refactoring this.

If the questions and answers were truly meaningful objects of their own, (with question/answer text, relations between questions and answers, accepted answers, etc.) rather than just vote objects, you might consider adding a trait to provide voting functionality (including implementation) to each such class that needs this functionality.

Since that is not the case here, I would suggest using a base Vote class (possibly abstract) where you can provide common implementations for most of your methods. This functionality could be inherited by QuestionVote and AnswerVote (or similar) classes and overridden as needed for each type of vote.

You are doing absolutely nothing to validate the data being passed to your public methods. That means this code is extremely fragile and likely to be put into unexpected states.

Take your constructors for example. They just blindly accept whatever is passed to them and sets the passed data as the DB connection to be used by the instantiated object.

Since $dbh (a poor variable name by the way) is an object, you should be able to type hint the parameter like this.

// note more meaningful property and variable names
public function __construct(PDO $pdo) {
    $this->pdo = $pdo;
}

This guarantees that these objects are passed a valid dependency upon instantiation. If you try to pass anything else to the constructor, an exception will be thrown and the object will not be created. It fails loudly. This is desired behavior, as the calling code can then do whatever it needs to do to handle the failure condition, which would obviously include not trying to conduct any further operations against that object as would happen now with your current code.

Your setters probably also need validation. For example, perhaps you need to fail loudly if a non-integer (or integer value in string) is set for ID value. Perhaps you need to fail if an invalid vote type is attempted to be set.

I think you need to consider either passing all needed dependencies (id's, voter, vote type, etc.) to the constructor, or you need to validate these values are set before performing database queries which rely on them. I do question why caller needs to provide an id for inserting a vote though - is this not autoincrement field in table?

Without understanding how you are working with these objects in your application, it would be hard to determine which approach might be best. If you are treating these dependencies on the objects as immutable during the lifetime of the object, then passing dependencies to constructor probably makes the most sense. If the dependencies can change during lifetime of the object, then perhaps rather than a bunch of individual setters for these dependencies, you provide a single method to set the dependencies on the object where you can perform all validation necessary to make sure the object is set-up properly to work against the database.

Why does answer class use ID values as primary key while question class uses title?
You mingle qtitle, questiontitle and just title as nomenclature in the question class which seems extremely confusing.

These sorts of inconsistencies probably make it harder for you to see the fact that these classes should probably inherit from a common ancestor with properties like:

$id
$title
$voter
$vote_type

Odd variants in your property names like $answerId and $qTitle don't give you any additional information you need beyond $id and $title as it should already be clear as to whether you are working with an answer object or a question object.

The more I look at this, I wonder whether a single class could actually satisfy all your requirements as question vs. answer vs. [some other type] could just be ano

StackExchange Code Review Q#148751, answer score: 2