# Calculating the price of digital media

I have taken over maintenance of some code, and have restructured it after reading some documents about PSR-2:

<?php

function calc_p()
{
list($arg1,$arg2, $arg3) = func_get_args(); switch($arg1) {
case 'ebook':
$bp = 5;$tax = .08;
break;
case 'audio_book':
$bp = 5;$tax = .08;
break;
case 'text_book':
$bp = 7.5;$tax = .1;
break;
}
$bp = ($bp + ($bp*$tax));
//Calclate shipping
if($arg3 == 'California') { } else {$bp = $bp + 1; } if ($arg3 == 'Texas') {
$norm =$bp;
$bp =$norm + 5;
}
//final price format: $XX:XX$final_price = '$'.$bp;
if (strpos($final_price, '.')) {$num = (2 - (strlen(substr($final_price, strpos($final_price, '.') + 1))));

for ($num>0;$num--;) {
$final_price .= '0'; } } elseif (strpos($final_price, '.') == FALSE) {
$final_price =$final_price .'.00';
}
else {
}
return $final_price; }$totalprice = calc_p('text_book','oldversion','Texas');
echo '<p style="font-weight: bold;">Total Price: </p>' . $totalprice; ?>  I wanted to ask if what I've done is already enough to follow the coding standards, or if I am missing something. ## 2 Answers I've been waiting for this question to be reopened because the formatting issues here are quite extraordinary! I think I'll keep this post to a clean code/code styling post exclusively, that way others can touch on purpose and implementations. I think what's required here is a bullet point list, which I know is boring, but it's the easiest way to go line by line! Let's begin :) • Eeek! What in the world is calc_p()!? I immediately know nothing about your code. That's the opposite of what we want. Names should be meaningful and thorough. It looks like you're calculating a price, so a better name might be calculateBookPrice(). Functions are usually expected to be camelCase. • $arg1: bad name. $arg2: bad name. $arg3: bad name. What are these variables? We need description in these names!
• I had no idea this function needed variables for func_get_args(). You limit the arguments to three, so I'd say add three parameters to the function. That or have something such as ... $someArguments for a parameter. (Obviously with a name that fits your code) • What is $bp? I immediately have converted this name to "blood pressure". However, that doesn't make sense. Now your reader is confused. See how important useful variable names are?
• 'ebook', 5, .08: what do all these have in common? I don't know why you chose them. If you need to, create (constants, if you have a class) variables and apply these values to a named variable.
• I don't see a default to your switch. I need a default to function! It's best practice to use one anyways.
• What in the world is $bp = ($bp + ($bp*$tax));? Hows about $bp *= ++$tax;? It's shorter and takes advantage PHP's combined operators and pre-increment operator. However, even this isn't too pretty. Perhaps refactor the code before it...
• //Calclate shipping - this is a useless comment. Comment's should describe why.
• It'd be nice to get some place in between statements: if(, '$'.$bp.
• The condition for $arg3 == 'California' is empty, why not negate the comparison? ($arg3 != 'California')
• $bp =$bp + 1 --> $bp++ • Shouldn't you have a switch for the states you check against? • What is $norm? Are you referring to a person named Norman? Spell out a useful variable name!
• You may want to look into money_format or formatCurrency... That whole block is hideous!
• Some variables are like $final_price and some are like $totalprice! It's vital that you stay consistent!
• If he wants to implement his own formatting function, he should at least extract the method for that instead of having it in calc_p. Commented Jun 5, 2016 at 10:08

Most of the improvements Alex had already mentioned in his answer. So I just wrote something quickly down to visualise it a bit better ...

class Product {
const TYPE_AUDIO_BOOK = 'audio_ebook';
const TYPE_EBOOK      = 'ebook';
const TYPE_TEXT_BOOK  = 'text_book';
}

class Price {

private static $costMap = [ Product::TYPE_AUDIO_BOOK => [ 'cost' => 5, 'tax_rate' => self::TAX_RATE_FULL ], Product::TYPE_EBOOK => [ 'cost' => 5, 'tax_rate' => self::TAX_RATE_FULL ], Product::TYPE_TEXT_BOOK => [ 'cost' => 7.5, 'tax_rate' => self::TAX_RATE_REDUCED ] ]; private static$shippingCostMap = [
'California' => [ 'free_shipping' => true,  'extra_supplement' => 0 ],
'Texas'      => [ 'free_shipping' => false, 'extra_supplement' => 5 ]
];

const TAX_RATE_FULL    = 0.8;
const TAX_RATE_REDUCED = 0.1;

public static function calculatePrice($productType,$destinationState = null)
{
if (! isset(self::$costMap[$productType])) {
throw new \RuntimeException(sprintf('Unsupported product type "%s" given.', $productType)); }$basePrice = self::$costMap[$productType]['cost'] +
(self::$costMap[$productType]['cost'] * self::$costMap[$productType]['tax_rate']);

return $basePrice + self::calculateShippingCost($destinationState);
}

private static function calculateShippingCost($destinationState = null) {$cost = isset(self::$shippingCostMap[$destinationState]) && self::$shippingCostMap[$destinationState]['free_shipping']
? 0
: 1;

return isset(self::$shippingCostMap[$destinationState]['extra_supplement'])
? $cost += self::$shippingCostMap[$destinationState]['extra_supplement'] :$cost;
}
}

setlocale(LC_MONETARY, 'en_US.UTF-8');

echo money_format('%.2n', Price::calculatePrice(Product::TYPE_TEXT_BOOK, 'Texas')) . "\n";      // $14.25 echo money_format('%.2n', Price::calculatePrice(Product::TYPE_TEXT_BOOK, 'California')) . "\n"; //$8.25

• Why aren't you using const for the cost as well as the tax rate? Aren't you missing a key word such as array() ? Commented Jun 6, 2016 at 12:42
• @pacmaninbw In the given example it might be arguable to use constant values for the cost as well. However because the cost of a product is likely to differ from product to product you would end up with tons of constants which may only be used once. In php you can declare an array using the array keyword or just two square brackets. php.net/manual/en/language.types.array.php Commented Jun 6, 2016 at 17:02
• Some older computers such as my macbook from 2010 may not be running 5.4. Commented Jun 7, 2016 at 12:22