sgry / svnhue

Do What The F*ck You Want To Public License
6 stars 0 forks source link

Multiple color options #2

Closed janmartenjongerius closed 11 years ago

janmartenjongerius commented 11 years ago

Allow for HueState->setColor to get fed an associative array of options. Let the method iterate over the array, calling itself for each entry, using the key as first parameter and the value as the second parameter.

It's simply a good convention, since this allows for cleaner coding on the user side of things.

Some pseudo code:

<?php

public function setColor($Ai_Colormode, $Av_Color = null)
{
  if (empty($Av_Color) && is_array($Ai_Colormode)) {
    $options = $Ai_Colormode;    

    foreach ($options as $Ai_Colormode => &$Av_Color) {
      if (!$this->setColor($Ai_Colormode, $Av_Color) {
        // The preferred method would be to uncomment the following code:
        // throw new InvalidArgumentException(
        //   'Invalid argument or whatever message you normally throw in here'
        // );

        // Yet, I assume this will do, referencing your implementation of surrounding code.
        return false;
      }
    }    

    return true;
  }

  ...

}
sgry commented 11 years ago

Thanks for your input!

I am not exactly sure what you mean :) What would the array look like that will get passed to the method?

I intended the HueState class to only hold 1 color at a time. Since a Hue object can only hold 1 HueState object. The color in the HueState object applies to each group or light that is set in the Hue object. If you want different colors for different lights or groups you need to create a new Hue object.

Or did you mean to pass only 1 Argument instead of the 2 $Ai_Colormode and $Av_Color? For example like this:

$Aa_Color['mode'] = 'xy';
$Aa_Color['xy'] = array(0.123, 0.456);

$Lo_Hue->setState($Aa_Color);

Cheers Sven

janmartenjongerius commented 11 years ago

Indeed, it would enable you to combine the following part of your readme into a single method call.

Your code now:

<?php

// Set Color
// Only 1 colormode is possible per HueState object
$Lo_HueState->setColor('name', 'white');
$Lo_HueState->setColor('hue', array(200, 127)); // Array: hue, sat
$Lo_HueState->setColor('ct', 400); // Int: ct
$Lo_HueState->setColor('xy', array(0.143, 0.451)); // Array: x, y

Your code after:

<?php

// Set Color
// Only 1 colormode is possible per HueState object
$Lo_HueState->setColor(
  array(
    'name' => 'white',
    // Hue, Sat.
    'hue' => array(200, 127),
    'ct' => 400,
    // X, Y
    'xy' => array(0.143, 0.451)
  )
);
sgry commented 11 years ago

I see your point. A physical Hue light can only have 1 colormode so only 1 colormode is possible per HueState object. Maybe i should make that line bigger in the example comment ;-)

So it's either

$Lo_HueState->setColor('name', 'white');

or

$Lo_HueState->setColor('hue', array(200, 127)); // Array: hue, sat

or

$Lo_HueState->setColor('ct', 400); // Int: ct

or

$Lo_HueState->setColor('xy', array(0.143, 0.451)); // Array: x, y

Not all 4 :)

janmartenjongerius commented 11 years ago

ahh. That makes sense. It was a bit obscure indeed.