A color class and a CRUD class for it

Question

Is it better to have only one class color and include the functions from the second class in the 1st one? Or leave them apart?

Why is one better than the other or vice versa? The design pattern is important to me, so why choose one over the other?

Is there a problem if, let's say, we have the function create_color in which we instantiate the class itself like new color()?

class color
{
private $id = NULL; 
private $name = '';
private $rgb = NULL; 
private $cmy = NULL;
private $wavelength = NULL; 
private $frequency = NULL; 
public function __construct($name, $rgb, $cmy, $wavelenght, $frequency)
{
setName($name);
setRGB($rgb);
setCMY($cmy);
setWavelength($wavelength);
setFrequency($frequency);
}
public function __destruct()
{
}
public function setName($name)
{
$this->name=$name;
}
public function setRGB($rgb)
{
$this->rgb=$rgb;
}
public function setCMY($cmy)
{
$this->cmy=$cmy;
}
public function setWavelength($wavelength)
{
$this->wavelength=$wavelength;
}
public function setFrequency($frequency)
{
$this->frequency=$frequency;
}
public function getId()
{
return $this->id;
}
public function getName()
{
return $this->name;
}
public function getRGB()
{
return $this->rgb;
}
public function getCMY()
{
return $this->cmy;
}
public function getWavelength()
{
return $this->wavelength;
}
public function getFrequency()
{
return $this->frequency;
}
public function toJSON()
{
return "{'id':'".$this->id."', 'name':'".$this->name."', 'rgb':'".$this->rgb."', 'cmy':'".$this->cmy."', 'wavelength':'".$this->wavelength."', 'frequency':'".$this->frequency."'}";
}
public function toCSV()
{
return $this->id . ", " . $this->name . ", " . $this->rgb . ", " . $this->cmy . ", " . $this->wavelength . ", " . $this->frequency;
}
public function toHTML()
{
return "<p>ID: " . $this->id . "</p><p>Name: " . $this->name . "</p><p>RGB: " . $this->rgb . "</p><p>CMY: " . $this->cmy . "</p><p>Wavelength: " . $this->wavelength . "</p><p>Frequency: " . $this->frequency . "</p>";
}
}

Second class:

class CRUD_color
{
public function create_color($parameters)
{
 $color=new color();
 $color->setName($parameter['name']);
 $color->setRGB($parameter['rgb']);
 $color->setCMY($parameter['cmy']);
 $color->setWavelength($parameter['wavelength']);
 $color->setFrequency($parameter['frequency']);
 $entitymanager->persist($color);
 $entitymanager->flush();
}
public function request_color($parameters)
{
 $color=$entitymanager->find($parameter['id']);
 echo $color->toJSON($parameter['name']);
}
public function update_color($parameters)
{
 $color=$entitymanager->find($parameter['id']);
 $color->setName($parameter['name']);
 $color->setRGB($parameter['rgb']);
 $color->setCMY($parameter['cmy']);
 $color->setWavelength($parameter['wavelength']);
 $color->setFrequency($parameter['frequency']);
 $entitymanager->persist($color);
 $entitymanager->flush();
}
public function delete_color($parameters)
{
 $color=$entitymanager->delete($parameter['id']);
}
}

Answer 1

Just from looking at both classes, CRUD_color is essentially putting methods on top of Color but those methods seem like something you'd want in Color. I would ask yourself what the purpose CRUD_color serves that Color should not. If there is no reason for the 2nd class, then get rid of it.

I would recommend overloading the __get() operator and get rid of the get methods, it'll clean things up a little bit in your first class.

Also I wouldn't have create_color accept an array of parameters, instead be explicit with what the values need. If you don't want to take this advice, then at least do some error checking for when those parameters don't exist in the array.

...Also I think CRUD_color should probably be static, or at least right now it seems like you are treating it as a static class. If you want it to act like a class which is inheriting from Color I would instead have a constructor and initialize a Color object which is a data-member of CRUD_color.

• Noooooob: I want to have only one class but am unsure if that's right. The second class handles DB storage for color. If I have to do that in color then I have two options: code the logic of inserts right in the constructor of the class so all my objects are then also saved in the DB (which seems like a bad idea), or write a save() function which shall make the INSERT. I see here also a problem that causes the class to become more complicated since you need to have flags for database access etc. And i have no idea how to code a read from the database inside of my own class.
• CD001: @Noooooob with standard MVC principles your color model should probably comprise 3 classes, the colour itself, a class to talk to the database and a sort of interface layer that the application uses to talk to the other two - this might be of interest : stackoverflow.com/questions/5863870/…