I am quite new to OOP idea and and still confused about some concepts. I have written this script to practice OOP, but I am very suspicious of this script, since I feel like I am "inventing" a new way of using OOP that I have never seen others doing it.
My situation: I have a long <form>
in form.php
, and when that <form>
is submitted, some data(arrays) need to be handled/encoded in a certain way before storing into the db. When retrieving those data from db, I need to parse the encoded data before output onto the browser.
So I decided to break "the way to handle data" in this script into 3 components: Encode
, Decode
, Query
, and make each into a class.
Now I have class.php
that contains 3 classes:
class Encode {}
class Decode {}
class Query {}
And then I felt like I wanted to combine all three classes into 1 class, so I created:
class Form {
private $encode;
private $decode;
private $query;
public function __construct()
{
// instantiate the 3 other classes and store them into properties
}
public function encode()
{
return $this->encode;
}
}
and I call it like this:
$form = new Form()
$form->encode()->someMethod();
So my question is am I on the right track OR did I violate some basic principles about OOP?
It would be great if you could point out where I am misunderstanding OOP, thank you.
Try this code
class Form {
private $encode;
private $decode;
private $query;
public function __construct(Encode $encode, Decode $decode, Query $query)
{
$this->encode = $encode;
$this->decode = $decode;
$this->query = $query
}
public function encode()
{
return $this->encode;
}
}
$form = new Form(new Encode, new Decode, new Query)
$form->encode->someMethod();
In my opinion the encoding and decoding of the data has nothing to do with the form. You even mention it on your question:
some data(arrays) need to be handled/encoded in a certain way before storing into the db
So imo the encoding should be done at the database layer and at the very least not at the form layer.
Also from your example you are reaching through the Form
object to eventually do stuff with some encoder object:
$form->encode()->someMethod();
Why do you need access to the encoder from outside the Form
?
Finally you are also stating in a comment in your Form
constructor that you can // instantiate the 3 other classes and store them into properties
. I wouldn't not do this like that. Because it introduces tight coupling between the 3 other classes and the thing (in the case Form
) you instantiate it in. Making your code harder to debug and maintain, introduces hidden dependencies (I cannot see it uses e.f. Encoder
by looking at the method signature) and makes your code harder to test.
An example of what the above looks like:
class SomeDatabaseClass
{
const ENCODED_FIELDS = [
'field1'
];
private $dbConnection;
private $encoder;
private $decoder;
public function __construct(\PDO $dbConnection, Encoder $encoder, Decoder $decoder)
{
$this->dbConnection = $dbConnection;
$this->encoder = $encoder;
$this->decoder = $decoder;
}
public function getData()
{
$stmt = $this->dbConnection->query('SELECT field1, field2 FROM the_fields');
return $this->decodeFields($stmt->fetchAll());
}
public function saveData($fields)
{
$fields = $this->encodeFields($fields);
// store data in db
}
private function decodeFields($fields)
{
foreach ($fields as $name => $value) {
if (!in_array($name, self::ENCODED_FIELDS, true)) {
continue;
}
$fields[$name] = $this->decoder->decode($value);
}
return $fields;
}
private function encodeFields($fields)
{
foreach ($fields as $name => $value) {
if (!in_array($name, self::ENCODED_FIELDS, true)) {
continue;
}
$fields[$name] = $this->encoder->encode($value);
}
return $fields;
}
}
This way the form doesn't have to know about encoding / decoding data for a purpose (storing and retrieving data from the db) it isn't responsible for.
And you have decoupled the encoder/decoder from the class making your code more reasonable and testable.