RGB <-> HSL converter + hue shifting

My input is basically a .png image from the img/ directory (I figured out that I should parse it, because there is an exploit possible at the moment) and a color which I want to be applied to the image.

The RGB color of every pixel from the image and my input RGB color are being converted to HSL, the H value from my input RGB is merging with the pixel S and L values. This is then being converted back to RGB. My output is the picture with the new colors.

shifthue($image, $mode, $R, $G, $B)  
$image = your PNG image  
$mode = 0 just shifts the hue of the colors, but 1 will also colorize the greys  
$R, $G, $B = R, G and B value the color is thaken from.

I don't really have much programming experience and this is the biggest thing I wrote so far. I am looking for any kind of feedback here!

Demo available here.

Convert.php

```
255 ? 255 : $key);
return $key;
}

function rgb2hsl($R = 0, $G = 0, $B = 0){ //formulas taken from here: http://www.rapidtables.com/convert/color/rgb-to-hsl.htm
$R = validatergb($R);
$G = validatergb($G);
$B = validatergb($B);

$R /= 255.0;
$G /= 255.0;
$B /= 255.0;

$Cmax = max([$R, $G, $B]);
$Cmin = min([$R, $G, $B]);
$Cdelta = $Cmax - $Cmin;

if ($Cdelta == 0){
$H = 0;
} else {
switch ($Cmax){
case $R:
$H = fmod((($G-$B)/$Cdelta),6);
break;
case $G:
$H = (($B-$R)/$Cdelta)+2.0;
break;
case $B:
$H = (($R-$G)/$Cdelta)+4.0;
break;
default:
$H = 0;
}
}
$H *= 60;
$L = ($Cmax + $Cmin) / 2;
$S = ($Cdelta == 0 ? 0 : ($Cdelta/(1-(abs(2$L - 1)))))100;

$L *= 100;

return array($H, $S, $L);
}

function hsl2rgb($H = 0, $S = 0, $L = 0){ //form

Been a while since I coded PHP, but I'll give it a shot. I'll stick to the conversion functions for this (don't want to re-read the imagemagick PHP API).

Overall, I found the following things:

-
Naming.

Your function names are alloneword. Use snake_case instead, like most of PHP tends to do. Also, you have a lot of variables that are UPPERCASE, like $R, $G, and $B, though there's really no reason. Personally, I'd stick to lowercase or perhaps camelCase for variables. Classes are named with PascalCase and constants are ALL_CAPS_AND_UNDERSCORES, but nothing else should start with a capital letter.

-
Whitespace.

Use more spaces, please. It makes it a lot easier to read a line like fmod((($G-$B)/$Cdelta),6); if you give a bit of breathing room.

-
Parentheses.

Use fewer, please. For instance, you've wrapped all your ternaries, though they don't all have to be. echo $x ? $y : $z works the same as echo ($x ? $y : $z). Also, you're unnecessarily putting parentheses around function calls, like this:

($Cdelta/(1-(abs(2*$L - 1)))))*100

That is a lot of close-parens in a row there. But the same can be written as:

100 * $Cdelta / (1 - abs(2 * $L - 1))

Multiplication is handled before division, and the abs call already has parentheses, so no need to wrap it further. Also note the extra bit of spacing.

Another example of the last two points:

} elseif (($H >= 1)&&($H < 2)){

Why have a space after elseif, but no space between the close-parenthesis and the opening brace (){)? Why wrap the comparisons in parentheses? The same could be written more readably as just:

} elseif ($H >= 1 && $H < 2) {

More specific review items (just going from the top):

function validatergb($key){  //doesnt really need to be a own function

Firstly, spellcheck your comments. Secondly, if it doesn't need to be its own function, why is it its own function? Thirdly, this function does not "validate" anything. It clamps a value to the range 0-255. And lastly, why the name $key? If anything, a single component color of RGB is called a "channel" or simply a "value".

Here's my interpretation:

function clamp_rgb_channel($value) {
    return min(max($value, 0), 255);
}

With regard to rgb2hsl, here's my take. Just general clean-up and some mild refactoring:

function rgb2hsl($r = 0, $g = 0, $b = 0) {
    // you may inline the min(max()) clamping, if you
    // don't want to use a separate function for it
    $r = clamp_rgb_channel($r) / 255.0; 
    $g = clamp_rgb_channel($g) / 255.0;
    $b = clamp_rgb_channel($b) / 255.0;

    // note: No need to use an array; min and max are variadic
    $maxC = max($r, $g, $b);
    $minC = min($r, $g, $b);
    $deltaC = $maxC - $minC;

    if ($deltaC == 0){
        $h = 0;
        $s = 0; // we might as well set this too
    } else {
        $s = $deltaC / (1 - abs(2 * $l - 1));
        switch ($maxC){
            case $r:
                $h = fmod(($g-$b) / $deltaC, 6);
                break;
            case $g:
                $h = (($b-$r) / $deltaC) + 2.0;
                break;
            case $b:
                $h = (($r-$g) / $deltaC) + 4.0;
                break;
            default:
                $h = 0;
        }
    }

    $l = ($maxC + $minC) / 2;

    // put all the final multiplication at the end; bookends
    // nicely with the initial division-by-255
    $h *= 60;
    $l *= 100;
    $s *= 100;

    return array($h, $s, $l);
}

I'll leave off here, since some of these points can be applied to the rest of the code as well.

Only thing to note is that you sometimes use ternaries where a simple if would be clearer, because there's only one branch. For instance:

$H = ($H < 0 ? $H += 360 : $H);

For one, those parentheses are unnecessary, but then so is the ternary. It'd be simpler to just say:

if ($H < 0) {
    $H += 360;
}

Or:

if ($H < 0) $H += 360;

But I recommend always using braces, even for one-liners. An extra keystroke or two never hurt anyone, and only makes the code absolutely unambiguous.

Of course, there's the risk that $H is waaay below zero, in which case adding 360 still wouldn't bring it above zero. Hence, you'll want to use $H % 360 instead. You might even do something like this, if you still want the ternary:

$value %= 360;
$value >= 0 ?: $value += 360;

That ?: is so-called the Elvis operator, by the way. I wouldn't actually recommend coding like this - it's fairly convoluted - but I couldn't help but use the Elvis operator somewhere.

StackExchange Code Review Q#109392, answer score: 2