HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Converting UK drams to ounces and pounds in PHP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
dramsouncesphppoundsandconverting

Problem

I have written this little function to convert UK weight drams (dr) to ounces (oz) and pounds (lbs). As far as my research goes, 1 lbs = 16 oz and 1 oz = 16 dr. I need the function to return an array of lbs, oz and dr.

Am I doing it right? Or is there a better way to do it? Has my method got any flaws?

function fromDrams($drams){

    // Nullify pounds
    $ouncesAndDrams = $drams % 256;

    $lbs = (int) ($drams / 256);
    $oz  = (int) ($ouncesAndDrams / 16);
    $dr  = $ouncesAndDrams % 16;

    return ['lbs' => $lbs, 'oz' => $oz, 'dr' => $dr];
}


It does return good values when tested:

fromDrams(273) = ['lbs' => 1, 'oz' => 1, 'dr' => 1];
fromDrams(272) = ['lbs' => 1, 'oz' => 1, 'dr' => 0];
fromDrams(271) = ['lbs' => 1, 'oz' => 0, 'dr' => 15];
fromDrams(256) = ['lbs' => 1, 'oz' => 0, 'dr' => 0];
fromDrams(32) = ['lbs' => 0, 'oz' => 2, 'dr' => 0];
fromDrams(17) = ['lbs' => 0, 'oz' => 1, 'dr' => 1];
fromDrams(15) = ['lbs' => 0, 'oz' => 0, 'dr' => 15];

Solution

There are two things that stand out to me in your code. The first is that your integer cast seems a bit premature. When creating a general function like this, you probably do not want to round the answer, since 1.5 oz is a perfectly valid weight. A user of your function can then cast the resulting values if this is required.

The second is that your conversion values are magic numbers. These should be in constants with names that express the meaning of the number.

A class would be perfect to store the logic and constants. A Weight class would probably be a well-suited name. An Weight object can then hold the amount of weight in one specific unit and provide public methods to retrieve the weight in other units.

The Weight class that I have created here provides three static "Builder/Factory" methods to create a Weight object from. The constructor has been made private to support the use of the more expressive static function. The reasoning behind this, is that $weight = Weight::fromDrams(16); is a lot easier to understand than $weight = new Weight(16);.

Furthermore, the class provides the three public methods to retrieve the given weight for a specific unit.

class Weight {
    const POUNDS_TO_OUNZES = 16;
    const OUNZES_TO_DRAMS = 16;

    /**
     * @var float
     */
    private $drams;

    public static function fromPounds ($pounds) {
        return self::fromOunzes($pounds * self::POUNDS_TO_OUNZES);
    }

    public static function fromOunzes ($ounzes) {
        return self::fromDrams($ounzes * self::OUNZES_TO_DRAMS);
    }

    public static function fromDrams ($drams) {
        return new static($drams);
    }

    private function __construct ($drams) {
        $this->drams = $drams;
    }

    public function inPounds () {
        return $this->inOunzes() / self::POUNDS_TO_OUNZES;
    }

    public function inOunzes () {
        return $this->inDrams() / self::OUNZES_TO_DRAMS;
    }

    public function inDrams () {
        return $this->drams;
    }
}


If you still require a very specific function that returns an array of 'lbs', 'oz' and 'dr' like your question says, than this Weight class can be used in conjunction with it as such:

function fromDrams ($drams) {
    $weight = Weight::fromDrams($drams));

    return [
        'lbs' => (int) $weight->inPounds(),
        'oz' => (int) $weight->inOunzes(),
        'dr' => (int) $weight->inDrams(),
    ];
}


Here you see the Weight class not performing any rounding itself and letting the user of the class perform these when required.

This Weight class is now much more versatile than your original piece of code and can be used in many other situations. For example a website that provides conversion of an amount of weight for a specific unit to the respective weights for the other three units.

if (!empty($_POST["amount_in_drams"]))
    $weight = Weight::fromDrams($_POST["amount_in_drams"]);

if (!empty($_POST["amount_in_ounzes"]))
    $weight = Weight::fromOunzes($_POST["amount_in_ounzes"]);

if (!empty($_POST["amount_in_pounds"]))
    $weight = Weight::fromPounds($_POST["amount_in_pounds"]);

echo $weight->getPounds() . " pounds, ";
echo $weight->getOunzes() . " ounzes or ";
echo $weight->getDrams() . " drams.";

Code Snippets

class Weight {
    const POUNDS_TO_OUNZES = 16;
    const OUNZES_TO_DRAMS = 16;

    /**
     * @var float
     */
    private $drams;

    public static function fromPounds ($pounds) {
        return self::fromOunzes($pounds * self::POUNDS_TO_OUNZES);
    }

    public static function fromOunzes ($ounzes) {
        return self::fromDrams($ounzes * self::OUNZES_TO_DRAMS);
    }

    public static function fromDrams ($drams) {
        return new static($drams);
    }

    private function __construct ($drams) {
        $this->drams = $drams;
    }

    public function inPounds () {
        return $this->inOunzes() / self::POUNDS_TO_OUNZES;
    }

    public function inOunzes () {
        return $this->inDrams() / self::OUNZES_TO_DRAMS;
    }

    public function inDrams () {
        return $this->drams;
    }
}
function fromDrams ($drams) {
    $weight = Weight::fromDrams($drams));

    return [
        'lbs' => (int) $weight->inPounds(),
        'oz' => (int) $weight->inOunzes(),
        'dr' => (int) $weight->inDrams(),
    ];
}
if (!empty($_POST["amount_in_drams"]))
    $weight = Weight::fromDrams($_POST["amount_in_drams"]);

if (!empty($_POST["amount_in_ounzes"]))
    $weight = Weight::fromOunzes($_POST["amount_in_ounzes"]);

if (!empty($_POST["amount_in_pounds"]))
    $weight = Weight::fromPounds($_POST["amount_in_pounds"]);

echo $weight->getPounds() . " pounds, ";
echo $weight->getOunzes() . " ounzes or ";
echo $weight->getDrams() . " drams.";

Context

StackExchange Code Review Q#115817, answer score: 2

Revisions (0)

No revisions yet.