# Best practice for converting minutes and seconds in seconds

I have 2 input fields to input the duration of a CD song. The first input is for minutes and the second is for seconds

When submitting the form, I must insert in the db the duration either in seconds (minutes + seconds) or NULL. Here's my validation formula:

THIS CODE HAS BEEN EDITED WILL ALL THE GIVEN TIPS

``````// HTML
<input id="min" name="min" type="text" value="...">
<input id="sec" name="sec" type="text" value="...">

// prepare/sanitize variables for function
\$min = ltrim(trim(\$_POST['min']), '0');
\$sec = ltrim(trim(\$_POST['sec']), '0');

\$warning = array();
\$duration_in_secs = validate_duration(\$warning, \$min, \$sec, true);

function validate_duration(&\$warning, \$min, \$sec, \$required = false) {

\$duration = NULL;

if (empty(\$min) && empty(\$sec)) {

if (\$required) {
\$warning['duration'] = "The duration is a required field";
}

} else {

if (!ctype_digit(\$min) || (int)\$min < 0 || (int)\$min > 59) {
\$warning['min'] = 'The minutes must contain a positive numeric value between 0 and 59';

} else {
\$duration += (int)\$min * 60;
}

if (!ctype_digit(\$sec) || (int)\$sec < 0 || (int)\$sec > 59) {
\$warning['sec'] = 'The seconds must contain a positive numeric value between 0 and 59';

} else {
\$duration += (int)\$sec;
}
}

return \$duration;
}
``````
-
`\$warning` doesn't seem to be accessible outside of the function? –  Corbin Sep 30 '12 at 6:40
true. i edited my code... –  Marco Sep 30 '12 at 15:14
Is the maximum length of the song 59 minutes 59 seconds? And what if the user wants to input 2.5 minutes as `150 seconds` (maybe they are getting that info in total seconds from a different source/app and don't want to make the conversion)? –  frozenkoi Oct 2 '12 at 8:23

Here is some tips:

• Name of function should be clearer
• Try to avoid ctype_digit usage, because it may produce unexpected results - ctype_digit(42) returns FALSE
• Because you working with integers, you should cast values to integers, note that inputs are always strings.
• You must check for negative values, also.
• Second can be 0 for example: 2 min and 0 sec. Also, some songs can lasts lower then minute (Classic songs)
• Consider that some CD song may have duration more then 60 min, CD medium can store 80min and it can be one song, some mix or classic songs. So, maybe you should avoid minute check.

``` ```

``````/**
* Validate song duration
* @param \$min
* @param \$sec
* @param \$required
* @return int|NULL
*/
function validate_duration(\$min, \$sec, \$required = false) {
global \$warning;

\$min = (int) \$min;
\$sec = (int) \$sec;

if(\$required && \$min == 0 && \$sec == 0) {
\$warning['duration'] = "The duration is a required field.";
return NULL;
}

if(\$sec < 0 OR \$sec > 59) {
\$warning['sec'] = 'The seconds must be between 0 and 59';
}

if(\$min < 0 OR \$min > 60) {
\$warning['min'] = 'The minutes must be between 0 and 60';
}

// for songs bigger then 60 minutes
if(\$min == 60 && \$sec > 0) {
\$warning['duration'] = "Song must last more then one second and maximum 60 minutes.";
}

return !empty(\$warning) ? \$min * 60 + \$sec : NULL;
}
``````
``` ```

-
1. I'm not familiar with PHP too much but globals are usually considered harmful. I don't know what's the best practice in PHP but passing the `\$warning` argument by reference seems a good idea here.

2. When a field is not posted you get various PHP notices. I guess it's better to avoid these notices:

``````PHP Notice:  Undefined index: min in /tmp/x.php on line ...
PHP Notice:  Undefined index: sec in /tmp/x.php on line ...
``````

A robust code should handle these cases.

3. `tmpsec` and `min_in_sec` are not initialized in every code paths, so you can get notices about these too when there is no `min` key in the `\$_POST` array:

``````PHP Notice:  Undefined variable: tmpsec in /tmp/x.php on line 24
PHP Notice:  Undefined variable: min_in_sec in /tmp/x.php on line 2
``````

I think it's cleaner to initialize all variables. It also could help debugging since you don't have to ignore lots of PHP notice messages when you are looking for an uninitialized global variable.

4. `tmpsec` and `min_in_sec` could be omitted if you reorganize the code to not count anything until it is sure that there isn't any input error. (See below.)

5. A small note: The function is a little bit inconsistent: it allows `60` as minute but doesn't as seconds. It would be cleaner and more common to set one hour as hours = 1, minutes = 0 and seconds = 0.

``````\$warning = array();

function val_duration(&\$warning, \$min, \$sec, \$required = false) {
if (empty(\$min) && empty(\$sec)) {
if (\$required) {
\$warning['duration'] = "The duration is a required field";
}
return NULL;
}

if (!ctype_digit(\$min) || \$min > 61) {
\$warning['min'] = 'The minutes must contain a positive numeric value between 1 and 60';
}
if (!ctype_digit(\$sec) || \$sec > 59) {
\$warning['sec'] = 'The seconds must contain a positive numeric value between 1 and 59';
}
if (!empty(\$warning)) {
return NULL;
}

return \$min * 60 + \$sec;
}
``````
-
val_duration(\$warning, 1, 1) will produce warning. Also var_duration(\$warning, "-1", "10") will produce -70. See my answer. –  sasa Sep 30 '12 at 20:13
@sasa: Thanks, I've not noticed this bug (it's in the original code too). Maybe I'm too tired but I could not reproduce the second one. It puts a message to the `warning` array and returns `NULL` instead of only returning `-70`. –  palacsint Oct 1 '12 at 17:09