Asking 2 values, feet and inches, and it converts them to centimeters

Question

I was doing an exercise in PHP, and it works perfectly, no problems. But I think that maybe I coded it so long, and I think it can be shorter than it is now, but I don't know how to make it shorter and that works exactly the same way.

I will explain what this code is doing for you so it's faster if you are going to help (if you want a deeper explanation, you can translate the commented section because that's the wording of the exercise): first of all, I have a PHP file and a html file with a form. The form it's asking for 2 values (feet and inches), for converting them to centimeters. Feet value must be integer, greater than or equal 0. Inches must be integer or decimal, and greater than or equal 0.

<?php
    //Realice un formulario que introduzca dos valores (pies y pulgadas) y los convierta a
    //centímetros. Los pies deben ser un número entero mayor o igual que cero. Las pulgadas
    //son un número entero o decimal mayor o igual que cero. Un pie son doce pulgadas y una
    //pulgada son 2,54 cm.

    $converPC = 30.48;
    $converPlC = 2.54;
    $pies = trim(htmlspecialchars(strip_tags($_REQUEST["pies"]), ENT_QUOTES, "UTF-8"));
    $pulgadas = trim(htmlspecialchars(strip_tags($_REQUEST["pulgadas"]), ENT_QUOTES, "UTF-8"));
    if ( (!empty($pies)) && (!empty($pulgadas)) ) {
        if ( (is_numeric($pies)) && (is_numeric($pulgadas)) ) {
            //Apartado de los pies
            if ( (filter_var($pies, FILTER_VALIDATE_INT)) && ($pies >= 0) ) {
                $resultadoPI = $pies*$converPC;
                print "<b>CONVERSIÓN PIES - CENTÍMETROS</b><br/>";
                print "$pies pies son $resultadoPI centímetros<br/><br/>";
            } else {
                print "<b>Error en pies</b><br/>";
                print "Debe introducir un número entero mayor o igual que cero<br/><br/>";
            }
            //Apartado de las pulgadas
            if ($pulgadas >= 0) {
                $resultadoPU = $pulgadas*$converPlC;
                print "<b>CONVERSIÓN PULGADAS - CENTÍMETROS</b><br/>";
                print "$pulgadas pulgadas son $resultadoPU centímetros";
            } else {
                print "<b>Error en pulgadas</b><br/>";
                print "Debe introducir un número mayor o igual que cero";
            }
        } else {
            print "Error, ambos valores deben ser numéricos";
        }
    } else {
        print "Para que todo funcione, debe rellenar TODOS los campos del formulario";
    }

?>

Answer 1

This code is pretty compact already so there isn't much room to reduce the size of it. Typically separating the error handling, calculations, and output of content are separated so you may want to look into that for code improvements but that won't make the code any smaller.

Better data sanitation

It's great that you do your best to sanitize user input before you use it. But, there's better ways to go about it. Since both inputs are expected to be floating point numbers you can use PHP's built in filter_var() with the FILTER_VALIDATE_FLOAT flag to sanitize the value to a floating point number:

$pies     = filter_var($_REQUEST["pies"], FILTER_VALIDATE_FLOAT, FILTER_FLAG_ALLOW_THOUSAND);
$pulgadas = filter_var($_REQUEST["pulgadas"], FILTER_VALIDATE_FLOAT, FILTER_FLAG_ALLOW_THOUSAND);

Combine/remove your IF statements

When you see an IF statement followed by another IF statement that's usually a sign that you could combine the two into one as all of them must be true for the following code to be executed:

if ( !empty($pies) && !empty($pulgadas) && (s_numeric($pies) && is_numeric($pulgadas) ) {

The above line can then be shortened thanks to the better sanitation used above. The checks to is_numeric are no longer needed since filter_var() will return a number or false which will be caught by the empty() checks. So you can now safely remove them:

if ( !empty($pies) && !empty($pulgadas) ) {

You can eliminate your check to see if $pies >= 0 by passing an extra flag to filter_var() to only allow positive numbers and zero.

if ( filter_var($pies, FILTER_VALIDATE_INT) && $pies >= 0 ) {

becomes

if ( filter_var($pies, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]]) ) {

You also forgot to add this check for $pulgadas.

Other notes

Use constants to store values that will remain the same and are unchangeable

Your variables containing the ratios for converting the measurements are better off set as constants than variables since they will remain the same and are unchangeable. (i.e. constant)

$converPC = 30.48;
$converPlC = 2.54;

becomes (notice the use of all capital letters as that is the expected format of constants in PHP)

define('CONVER_PC', 30.48);
define('CONVER_PLC', 2.54);

Omit closing PHP tag

When the closing tag is the last line of a PHP file you can safely omit and it is the standard practice as set forth by the PSR-2 coding standard for PHP. There are lots of good reasons to do this.

Use echo over print()

print() is an alias of echo but there are minor differences between the two. Although they don't come into play here, it is the PHP convention to use echo for outputting content.

Unnecessary parenthesis

If your IF statements you have parenthesis around each conditional. That is not necessary. You only need to use them when you need to clarify scope. When there's only one condition there is nothing that needs clarification.

Outcome

This code is untested but should give you the idea of what the comments above mean.

define('CONVER_PC', 30.48);
define('CONVER_PLC', 2.54);
$pies     = filter_var($_REQUEST["pies"], FILTER_VALIDATE_FLOAT, FILTER_FLAG_ALLOW_THOUSAND);
$pulgadas = filter_var($_REQUEST["pulgadas"], FILTER_VALIDATE_FLOAT, FILTER_FLAG_ALLOW_THOUSAND);
if ( !empty($pies) && !empty($pulgadas) ) {
    //Apartado de los pies
    if ( filter_var($pies, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]]) ) {
        $resultadoPI = $pies*CONVER_PC;
        echo "<b>CONVERSIÓN PIES - CENTÍMETROS</b><br/>";
        echo "$pies pies son $resultadoPI centímetros<br/><br/>";
    } else {
        echo "<b>Error en pies</b><br/>";
        echo "Debe introducir un número entero mayor o igual que cero<br/><br/>";
    }
    //Apartado de las pulgadas
    if ( filter_var($pulgadas, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]]) ) {
        $resultadoPU = $pulgadas*CONVER_PLC;
        echo "<b>CONVERSIÓN PULGADAS - CENTÍMETROS</b><br/>";
        echo "$pulgadas pulgadas son $resultadoPU centímetros";
    } else {
        echo "<b>Error en pulgadas</b><br/>";
        echo "Debe introducir un número mayor o igual que cero";
    }
} else {
    print "Para que todo funcione, debe rellenar TODOS los campos del formulario";
}

Answer 2

Main issues with this code are

1. It doesn't meet the requirements (so doesn't the code in the other answer).
2. Its result is not very useful if you ever try to actually use it.
3. Nearly half of this code just duplicates itself or plain useless.

So here goes your homework

$converPlC = 2.54;
$converPC = $converPlC * 12;

$pies = $_REQUEST["pies"];
$pulgadas = $_REQUEST["pulgadas"];

if (ctype_digit($pies) && $pies >= 0) {
    $resultadoPI = $pies*$converPC;
    print "<b>CONVERSIÓN PIES - CENTÍMETROS</b><br/>\n";
    print "$pies pies son $resultadoPI centímetros<br/><br/>\n";
} else {
    print "<b>Error en pies</b><br/>\n";
    print "Debe introducir un número entero mayor o igual que cero<br/><br/>\n";
    $resultadoPI = 0;
}
if (is_numeric($pulgadas) && $pulgadas >= 0) {
    $resultadoPU = $pulgadas*$converPlC;
    print "<b>CONVERSIÓN PULGADAS - CENTÍMETROS</b><br/>\n";
    print "$pulgadas pulgadas son $resultadoPU centímetros\n";
} else {
    print "<b>Error en pulgadas</b><br/>\n";
    print "Debe introducir un número mayor o igual que cero\n";
    $resultadoPU = 0;
}

It makes your code fixed but it doesn't look good. To make it better, we will need a sensible output and also we definitely should separate the calculations from the output. So here goes the refactored version

$converPlC = 2.54;
$converPC = $converPlC * 12;

$pies = $_REQUEST["pies"];
$pulgadas = $_REQUEST["pulgadas"];

if (ctype_digit($pies) && $pies >= 0 && is_numeric($pulgadas) && $pulgadas >= 0) {
    $resultadoPI = $pies*$converPC;
    $resultadoPU = $pulgadas*$converPlC;
    $resultado   = $resultadoPU + $resultadoPI;
    if ($resultadoPU && $resultadoPI) {
        $title = "CONVERSIÓN PIES Y PULGADAS - CENTÍMETROS";
        $message = "$pies pies y $pulgadas pulgadas son $resultado centímetros";
    } elseif ($resultadoPI) {
        $title = "CONVERSIÓN PIES - CENTÍMETROS";
        $message = "$pies pies son $resultado centímetros";
   } elseif ($resultadoPU) {
        $title = "CONVERSIÓN PULGADAS - CENTÍMETROS";
        $message = "$pulgadas pulgadas son $resultado centímetros";
    } else {
        $title = "Invalid input data";
        $message = "Enter at least one value";
    }
} else {
    $title = "Invalid input data";
    $message = "Input must be a positive number or zero";
}
?>
<b><?=$title?></b><br/>
<?=$message?><br/><br/>

Here you can see the immediate benefit of the separated output: when you will have to change the formatting, it would be done only once and without the hassle of escaping quotes and stuff.