trjhtr

I wrote this function to map value to a color

I am looking for some general feedback on how I can improve the efficiency of the function. and if there is better way to do it.

function getColor($value, $values_range, $opacity = '0.7') 

 $deltaS = ($values_range['max'] - $values_range['min']) / 2;
 $ds = ($value - $values_range['min']) / $deltaS;
 $nearest_color_index = floor($ds);
 $alpha = $ds - $nearest_color_index;

 $r = $g = $b = -1;
 $colors = [['r'=>255,'g' => 60,'b' => 40],
 ['r' => 255,'g' => 247,'b' =>40],
 ['r' => 12,'g' => 197,'b' => 17]];

 if ($nearest_color_index != 2) 
 $r = round($colors[$nearest_color_index]['r'] + $alpha * ($colors[$nearest_color_index + 1]['r'] - $colors[$nearest_color_index]['r']));
 $g = round($colors[$nearest_color_index]['g'] + ($alpha * ($colors[$nearest_color_index + 1]['g'] - $colors[$nearest_color_index]['g'])));
 $b = round($colors[$nearest_color_index]['b'] + $alpha * ($colors[$nearest_color_index + 1]['b'] - $colors[$nearest_color_index]['b']));

 return "rgba($r,$g,$b,$opacity)"; 

 else 
 $r = round($colors[$nearest_color_index]['r']);
 $g = round($colors[$nearest_color_index]['g']);
 $b = round($colors[$nearest_color_index]['b']);
 return "rgba($r,$g,$b,$opacity)";
 

 

Function name

The function name getColor isn't representative of what it does its better of being named createRgbaString (or something along those lines)

Return early

Your script returns in two different places, this should be changed so you exit the function early, it saves an else statement and a level of identing

Variable nameing style

Your variable naming style is not compatible with PSR2 but that isn't the end of the world.

Line length

Your line length exceeds 80 character line length, this is again not the end of the world but will make it hard to read for some people

Repeated calculation

Your repeatedly do the same calculation if the $index != 2 I would move this into a function so if it needs to be updated you only have todo it in one location

How I might write it

I have used terrible variable names, because I don't have the time do work out the complete logic of your script, so this should be unit tested if your going to use it

function getColor($value, $values_range, $opacity = '0.7')

 $deltaS = ($values_range['max'] - $values_range['min']) / 2;
 $ds = ($value - $values_range['min']) / $deltaS;
 $nearest_color_index = floor($ds);
 $alpha = $ds - $nearest_color_index;

 $r = $g = $b = -1;

 $colors = [
 ['r'=>255,'g' => 60,'b' => 40],
 ['r' => 255,'g' => 247,'b' =>40],
 ['r' => 12,'g' => 197,'b' => 17]
 ];

 $rNormal = getColorIndexPlusModifier($nearest_color_index, "r", 0, $colors);
 $gNormal = getColorIndexPlusModifier($nearest_color_index, "g", 0, $colors);
 $bNormal = getColorIndexPlusModifier($nearest_color_index, "b", 0, $colors);

 if ($nearest_color_index == 2) 
 return "rgba($rNormal,$gNormal,$bNormal,$opacity)";
 

 $rPlusOne = getColorIndexPlusModifier($nearest_color_index, "r", 1, $colors);
 $gPlusOne = getColorIndexPlusModifier($nearest_color_index, "g", 1, $colors);
 $bPlusOne = getColorIndexPlusModifier($nearest_color_index, "b", 1, $colors);

 $r = doCalculation($rNormal, $alpha, $rPlusOne);
 $g = doCalculation($gNormal, $alpha, $gPlusOne);
 $b = doCalculation($bNormal, $alpha, $bPlusOne);

 return "rgba($r,$g,$b,$opacity)";


function getColorIndexPlusModifier($colorIndex, $colorString, $toModifyBy, $colors)

 return round($colors[$colorIndex + $toModifyBy][$colorString]);

//TODO Rename to something meaningful 
function doCalculation($normalValue, $alpha, $plusOneValue)

 return round($normalValue + $alpha * ($plusOneValue - $normalValue));