Page 1 of 1
Class constants relating to directories/images
Posted: Tue Dec 05, 2006 6:39 pm
by Luke
I have a class called UCA_Member (which is a model within MVC) with two constants, STANDARD and PREFERRED... now I need to select an icon depending upon whether a value (from the database) is STANDARD or PREFERRED. Some day I may add another constant... something like SUBSTANDARD. What would be the best way to do that without making MORE work for myself in the futer ie: having to create a directory as well as adding the constant. Thanks!
The class looks like this (extremely simplified for your sake)
Code: Select all
<?php
class UCA_Member
{
protected $_table = "members";
const STANDARD = 1;
const PREFERRED = 2;
public function getIcon()
{
if($this->type == self::STANDARD)
{
// Not sure what to do
}
}
}
Maybe it would make more sense to do something like this?
Code: Select all
<?php
class UCA_Member
{
protected $_table = "members";
protected $_types = array('STANDARD', 'PREFERRED', 'SUBSTANDARD');
public function getIcon()
{
foreach($this->_types as $key => $type)
{
if($this->type == $key)
{
$icon = $type;
}
}
}
}
Posted: Tue Dec 05, 2006 7:15 pm
by John Cartwright
Code: Select all
<?php
class UCA_Member
{
protected $_table = "members";
protected $_types = array('STANDARD', 'PREFERRED', 'SUBSTANDARD');
public function getIcon()
{
$logo = in_array($this->type, $this->_types) ? $this->type : $this->_types[0];
}
}
I typically would take this course, although there isn't anything technically wrong with using constants.
Posted: Tue Dec 05, 2006 7:24 pm
by Luke
The only reason I am using constants is so I can do something like this...
Code: Select all
$type = new Form_select('type', 'Type: ');
$type->addOption('Standard', Member::STANDARD);
$type->addOption('Preferred', Member::PREFERRED);
$Form->addElement($type);
Although I really like this method because it allows me to do that, it seems like it's going to cause more work for me because now if I want to add another type, I have to go through and find all the places where the types are referenced and add another type:
Code: Select all
$type = new Form_select('type', 'Type: ');
$type->addOption('SubStandard', Member::SUBSTANDARD);
$type->addOption('Standard', Member::STANDARD);
$type->addOption('Preferred', Member::PREFERRED);
$Form->addElement($type);
and then add another case to my getIcon switch (if that's what I use)
Code: Select all
public function getIcon()
{
switch($this->type)
{
case self::PREFERRED;
$icon = 'preferred';
break;
case self::SUBSTANDARD;
$icon = 'substandard';
break;
case self::STANDARD:
default:
$icon = 'standard';
break;
}
return $icon;
}
Posted: Tue Dec 05, 2006 8:06 pm
by John Cartwright
You are breaking encapsulation by doing something like
Code: Select all
$type = new Form_select('type', 'Type: ');
$type->addOption('Standard', Member::STANDARD);
$type->addOption('Preferred', Member::PREFERRED);
$Form->addElement($type);
to which leads to code smell. Your form controller shouldn't be aware of variables found in your model.
Code: Select all
$type = new Form_select('type', 'Type: ');
$type->addOption('Standard', $this->model->getStatus('STANDARD'));
$type->addOption('Preferred', $this->model->getStatus('PREFERRED'));
$Form->addElement($type);
which promotes encapsulation.
Code: Select all
public function getIcon()
{
switch($this->type)
{
case self::PREFERRED;
$icon = 'preferred';
break;
case self::SUBSTANDARD;
$icon = 'substandard';
break;
case self::STANDARD:
default:
$icon = 'standard';
break;
}
return $icon;
}
is identical to the snippet I posted, sans the lower case, which can easily be fixed either by having your original values all lower case, or passing your return through strtolower()
Posted: Wed Dec 06, 2006 5:18 am
by Ollie Saunders
I don't think using constants breaks encapsulation. Why do we encapsulate at all? Mainly to avoid one bit of code meddling with another. You can't set a constant so I don't see what the problem is.
Oh Ninja how about you use a string? What do these constants mean anyway?
Posted: Wed Dec 06, 2006 7:52 am
by Jenk
the external referencing of the constant is what breaks encapsulation, not the constant it self.
Depending on the design of the rest of your application, ninja, I'd consider a settings object for the types.
Posted: Wed Dec 06, 2006 11:39 am
by Luke
OK, so let's say I go with this method... (yes, corridors is new--I guess it's relevant so I should include it)
In my table, I have a column called type and I have a column called corridor... I was hoping to just store a 4 digit (maximum) integer in each of these columns... then use the class to grab the icon based on what corridor and what type. The corridor determines the color, and the type determines the icon. I would prefer the colors and icons to be defined in the class... so with all that in mind, what do you guys think of this method?
(untested)
Code: Select all
<?php
class Member
{
static protected $_types = array('standard', 'preferred');
static protected $_corridors = array('highway 99', 'highway 70', 'interstate 5');
public function getType()
{
if(is_null($this->type)) return false;
$type = (in_array($this->type, self::$types)) ? self::$_types[$this->type] : self::$types[0];
return ucwords($type);
}
public function getCorridor()
{
if(is_null($this->type)) return false;
$corridor = in_array($this->corridor, self::$_corridors) ? self::$corridors[$this->corridor] : null;
return is_null($corridor) ? false : ucwords($corridor);
}
public static function getCorridors()
{
$corridors = array();
foreach(self::$_corridors as $corridor)
{
$corridors[] = ucwords($corridor);
}
return $corridors;
}
public static function getTypes()
{
$types = array();
foreach(self::$_types as $type)
{
$types[] = ucwords($type);
}
return $types;
}
public function getIcon()
{
switch($this->corridor)
{
case self::$_corridors[0]:
$color = 'green';
break;
case self::$_corridors[1]:
$color = 'yellow';
break;
case self::$_corridors[2]:
$color = 'pink';
break;
default:
$color = '#999999';
break;
}
switch($this->type)
{
case self::$_types[1]:
$icon = 'preferred';
case self::$_types[0]:
default:
$icon = 'standard';
}
return $icon . '_' . $color;
}
}
?>
And for the form generation...
Code: Select all
$corridors = Member::getCorridors();
$corridor = new Form_select('corridor', "Corridor: ");
$corridor->addOption(' - Select One - ', '');
$corridor->loadArray($corridors);
$Form->addElement($corridor);
Posted: Wed Dec 06, 2006 2:29 pm
by John Cartwright
You are mixing object and static context.
Code: Select all
public function getCorridor()
{
if(is_null($this->type)) return false;
$corridor = in_array($this->corridor, self::$_corridors) ? self::$corridors[$this->corridor] : null;
return is_null($corridor) ? false : ucwords($corridor);
}
Should return as a fatal error.
Posted: Wed Dec 06, 2006 3:48 pm
by Luke
This is how it is in my class now and it works fine...
Code: Select all
public function getCorridor()
{
if(is_null($this->corridor)) return false;
$corridor = in_array($this->corridor, self::$_corridors) ? self::$_corridors[$this->corridor] : null;
return is_null($corridor) ? false : ucwords($corridor);
}
$this->corridor is set on a member-by-member basis, self::_$corridors is a static array.
Posted: Wed Dec 06, 2006 10:26 pm
by John Cartwright
Why are you making them static? If you are creating the object why not just make them properties?
You must be calling getCorrider() through the object context, so why bother having similar methods use different context?
Posted: Thu Dec 07, 2006 1:08 am
by Chris Corbyn
Jcart wrote:You are mixing object and static context.
Code: Select all
public function getCorridor()
{
if(is_null($this->type)) return false;
$corridor = in_array($this->corridor, self::$_corridors) ? self::$corridors[$this->corridor] : null;
return is_null($corridor) ? false : ucwords($corridor);
}
Should return as a fatal error.
You can do that just fine. Accessing static data is globally fine. Accessing an object's data from a static member will not work without injecting this object into the function.
I use class constants for things which don't change. Typically I'll use them for configuration options:
Code: Select all
$swift = new Swift( ... , (Swift::NO_START | Swift::ENABLE_LOGGING));
I've used them in label definitions before too:
Code: Select all
abstract class Swift_Log_Base implements Swift_Log
{
/**
* A command type entry
*/
const COMMAND = ">>";
/**
* A response type entry
*/
const RESPONSE = "<<";
/**
* An error type entry
*/
const ERROR = "!!";
/**
* A standard entry
*/
const NORMAL = "++";
// ... snipp ....
/**
* Add a new entry to the log
* @param string The information to log
* @param string The type of entry (see the constants: COMMAND, RESPONSE, ERROR, NORMAL)
*/
abstract public function add($text, $label);
// ... snip ...
}
I don't see the harm in using them provided you're happy that you won't want to change the values dynamically.
Posted: Thu Dec 07, 2006 9:12 am
by John Cartwright
Accessing an object's data from a static member will not work without injecting this object into the function.
I know this, but why would you have two separate methods, very similar, use two different object context?
You already have the object, why not use it? Must cleaner interface IMO.
Posted: Thu Dec 07, 2006 11:12 am
by Luke
I did it so I can do this...
Code: Select all
$corridors = Member::getCorridors();
$corridor = new Form_select('corridor', "Corridor: ");
$corridor->addOption(' - Select One - ', '');
$corridor->loadArray($corridors);
$Form->addElement($corridor);