Page 1 of 1
Thinking in OOP
Posted: Thu May 29, 2003 5:13 pm
by JPlush76
I've been a proceedural programmer since I started (AS/400 SYSTEMS) and I'm trying to try and think more in the OOP sense for some things.
I have a shipping class I made and now that I think more about it I'm not sure I coded it correctly.
Basically I have a class for the various shipping methods we have available... ground, 2day, 3day, overnight, international, usps
and I have them set up as methods within my class so when I create the object it looks like...
Code: Select all
<?php
$ship = new Shipping($add1,$add2,$country,$state,$zip,$weight,$hazmat);
$ship->getLocation();
if($hazmat)
{
$ship->checkHazmat($hazmat);
$ship->setHazmatMSG('<BR><img src="images/hazard.gif"><font color="red"><B>Notice: You have one or more chemicals that are listed as an ORMD item. These items can only be shipped in the Continental United States via Ground shipping.</B></font>');
}
echo $ship->ground();
echo $ship->overnight();
echo $ship->fed2Day();
echo $ship->fed3Day();
echo $ship->fedInt();
echo $ship->usps();
echo $ship->error();
?>
so I'm thinking should I have created one parent class and made each shipping option a child of the parent class?
Posted: Fri May 30, 2003 9:09 am
by stuart
Its hard to tell unless you show the class script.
With my experience of OO in PHP (which is quite short) one thing I have learned is to keep classes, objects and methods to a minumum (if it makes sense to that is). Although it may look prettier to have everything all seperated, you may just be causing a lot more processing.
But as I said its hard to tell without seing the classes.
Posted: Fri May 30, 2003 11:34 am
by JPlush76
well its long but here goes...
Code: Select all
<?php
<?php
class Shipping
{
var $hazmat_message;
var $hazmat;
var $status;
var $h_weight;
var $real_weight;
var $zone;
// constructor - set variables
function Shipping($add1, $add2, $country, $state, $zip, $weight)
{
$this->add1 = $add1;
$this->add2 = $add2;
$this->country = $country;
$this->state = $state;
$this->zip = $zip;
$this->weight = $weight;
$this->usps_array = array("AA", "AE", "AK", "AP", "AS", "FM", "GU", "HI","MH", "MP", "PW", "VI", "PR");
$this->status = $this->getLocation();
$this->setWeight();
$this->setZone();
}
// METHODS ********************************************************************************
// STATUS CODES
// 1 = USA
// 2 = CANADA
// 3 = INTERNATIONAL
// ground shipping function
function ground()
{
if(($this->status == '1' || $this->status == '2') && !$this->checkPOBOX() && !$this->checkStates())
{
return $this->display('Standard Ground Shipping','FXG','').$this->hazmat_message;
}
}
// overnight shipping function
function overNight()
{
if($this->status == '1' && !$this->checkPOBOX() && !$this->checkStates() && !$this->hazmat)
{
$_SESSION['fed_1day'] = $this->getPrice($this->zone,'fed_1day',10);
return $this->display('Priority Overnight Shipping','FSO',$_SESSION['fed_1day']);
}
}
// 2 day express shipping
function fed2Day()
{
if($this->status == '1' && !$this->checkPOBOX() && !$this->checkStates() && !$this->hazmat)
{
$_SESSION['fed_2day'] = $this->getPrice($this->zone, 'fed_2day',10);
return $this->display('2 Day Shipping','FX2',$_SESSION['fed_2day']);
}
}
// 3 day express shipping
function fed3Day()
{
if($this->status == '1' && !$this->checkPOBOX() && !$this->checkStates() && !$this->hazmat)
{
$_SESSION['fed_3day'] = $this->getPrice($this->zone, 'fed_3day',10);
return $this->display('3 Day Shipping','FX3',$_SESSION['fed_3day']);
}
}
// International
function fedInt()
{
if(($this->status == '3' || $this->status == '2') && !$this->checkPOBOX() && !$this->checkStates() && !$this->hazmat)
{
$_SESSION['fed_int'] = $this->getPrice($this->zone, 'fed_int',3);
return $this->display('Priority International','FXI',$_SESSION['fed_int']);
}
}
// usps shipping function
function usps()
{
if(($this->checkPOBOX() || $this->checkStates()) && $this->status == '1' && !$this->hazmat)
{
$_SESSION['USPS'] = $this->getPrice($this->zone, 'fed_2day',-10);
return $this->display('USPS Priority','PIP',$_SESSION['USPS']);
}
}
// check to see if any errors apply to this order
function error()
{
if($this->status != '1' && $this->checkPOBOX())
{
echo '<font color="red"><B>ERROR:</B> PO BOXES are only issued in the United States, please re-check your information</font><BR><BR>';
}
if(($this->checkPOBOX() || $this->checkStates() || $this->status == '3') && $this->hazmat)
{
echo '<font color="red"><B>ERROR:</B> You have items in your cart that are listed as ORMD items, these cannot ship to PO BOXES or outside the Continental U.S. </font><BR><BR>';
}
if($this->checkPOBOX())
{
echo '<BR><font color="red"><B>PO BOXES:</B> We currently can only ship to PO BOXES via USPS Priority Shipping which is an additional charge. To avoid the additional charge please ship to a valid street address. </font><BR><BR>';
}
}
// ACCESSOR FUNCTIONS **********************************************************************
// determine based on the location which shipping options are available
function getLocation()
{
if($this->country == 'USA')
{
return '1';
}
elseif($this->country == 'CAN')
{
return '2';
} else {
return '3';
}
}
// display the appropriate radio buttons
function display($name,$value, $price)
{
if($price != '')
{
$price = ' + $'.$price;
}
if($value == $_SESSION['ship_method'])
{
$checked = 'checked';
}
return "<input class="radio" type="radio" name="ship_method" value="$value" $checked>$name$price<BR>";
}
// get the price of the shipping option
function getPrice($select, $from, $percent)
{
$result_ship = query_db("SELECT $select FROM $from WHERE weight = $this->h_weight");
$row_ship = @mysql_fetch_array($result_ship);
if($this->h_weight == 151 || $this->h_weight == 101){
$price = ($row_ship[$select]) * $this->weight;
} else {
$price = ($row_ship[$select]);
}
$fed_sur = sprintf("%01.2f", ($price / 100) * $percent);
$_SESSION[$from] = sprintf("%01.2f", ($fed_sur + $price));
return $_SESSION[$from];
}
// SET THE WEIGHT FOR PRICING
function setWeight()
{
if($this->country == 'USA')
{
if($this->weight > 150)
{
$this->h_weight = 151;
} else {
$this->h_weight = $this->weight;
}
} else {
if($this->weight > 100)
{
$this->h_weight = 101;
} else {
$this->h_weight = $this->weight;
}
}
} // end of setWeight function
// SET THE ZONE IF IN THE USA
function setZone()
{
if($this->status != 3)
{
$new_zip = substr($this->zip, 0,3);
$result = query_db("SELECT zone_num FROM zones WHERE $new_zip BETWEEN zip1 AND zip2");
$row = @mysql_fetch_array($result);
}
if($row){
$this->zone = ($row['zone_num']);
}
elseif($this->country == 'CAN')
{
$this->zone = 'A';
} else {
$result_c = query_db("SELECT countries_zone FROM countries WHERE countries_id = '$this->country'");
$row_c = @mysql_fetch_array($result_c);
$zone = ($row_c['countries_zone']);
$this->zone = $zone;
}
return $this->zone;
}
// CHECK IF USER IS SHIPPING TO A PO BOX
function checkPOBOX()
{
$pattern = array("box", "pbx", "bx");
foreach($pattern as $pat)
{
if (eregi($pat, $this->add1) || eregi($pat, $this->add2))
{
return TRUE;
}
}
}
// CHECK IF STATE IS FOR USPS
function checkStates()
{
if(in_array($this->state, $this->usps_array))
{
return TRUE;
}
}
// CHECK FOR HAZMAT FLAG
function checkHazmat($hazmat)
{
if($hazmat)
{
$this->hazmat = TRUE;
return TRUE;
}
}
// SET THE HAZMAT MESSAGE
function setHazmatMSG($message)
{
$this->hazmat_message = $message;
}
} // end of class
?>
?>
Posted: Sat May 31, 2003 9:37 pm
by McGruff
Didn't look at your code in detail.
In general, I'd say that a child/parent relationship is indicated if you can identify code common to all the childs - or at least to most of them. That should probably be written out once - in a parent - and the childs should do only what is unique to each child.
Don't forget that a parent constructor (if required) has to be explicitly called in the child .
Posted: Tue Jun 03, 2003 8:53 am
by BDKR
Hey J,
Have you ever heard of Responsibility Driven Design? It's the idea that components should be designed based on responsiblity. That of course implies that you need to identify the areas of responsibility in your system. In this way, you can design a system of components (they don't have to be objects mind you) where each has a distinct area of responsibility. That means greater flexibility for each component, the sytem, and less overhead in any single component.
Check it out on Google or something. Also try searching under RDD.
Cheers,
BDKR
Posted: Tue Jun 03, 2003 12:42 pm
by McGruff
stuart wrote:Its hard to tell unless you show the class script.
With my experience of OO in PHP (which is quite short) one thing I have learned is to keep classes, objects and methods to a minumum (if it makes sense to that is). Although it may look prettier to have everything all seperated, you may just be causing a lot more processing.
But as I said its hard to tell without seing the classes.
I've been reviewing my whole coding style recently and, although I used to think along the above lines, nowadays I'm much more inclined to split everything up into bits as much as possible.
With careful choice of variable names and function names, the "higher level" functions can read almost like pure english, becoming much more pared-down and elegant. Sub-functions carrying out specific tasks can often be re-used elsewhere and make it easier to update scripts: one change to one function rather than to many lines of code.
Classes can often help with this - depends on the script.
Posted: Wed Jun 04, 2003 4:14 pm
by JPlush76
I'm just trying to see if all the different shipping options would be thought of as methods to a shipping class or should they be children of a parent shipping class
Posted: Wed Jun 04, 2003 11:33 pm
by McGruff
Personally I'd be inclined to create childs. It maybe helps to understand the code if the common stuff is separated from the unique cases. I guess it's still separate as methods, it's just MORE separate as childs.
I'd probably have just a constructor in the childs and no other methods except maybe something to echo any output you need. The constructor would make all required calls to parent::methods - hides the modus operandi inside the class but makes the child less flexible. Childs are often one-trick ponies anyway.
It's also neater. In the calling script, you could end up with just:
$ob = new Child(...args...);
$ob->getOutput();
As I say I haven't studied your code carefully but that's my two cents.