Рефакторинг, если заявление со многими или

Question

Как рефакторинг в этих условиях сделать проще и чище?Логика совершенно разная для каждого if.

if(in_array($sides, [self::ALL_SIDES, self::HORIZONTAL_SIDES, self::TOP_SIDE])) {
    //do a()
}

if(in_array($sides, [self::ALL_SIDES, self::HORIZONTAL_SIDES, self::BOTTOM_SIDE])) {
    //do b()
}

if(in_array($sides, [self::ALL_SIDES, self::VERTICAL_SIDES, self::LEFT_SIDE])) {
    //do c()
}

if(in_array($sides, [self::ALL_SIDES, self::VERTICAL_SIDES, self::RIGHT_SIDE ])) {
    //do d()
}

Answer 1

class Test
{
    const TOP_SIDE = 'a';
    const BOTTOM_SIDE = 'b';
    const HORIZONTAL_SIDES = self::TOP_SIDE . self::BOTTOM_SIDE;

    const LEFT_SIDE = 'c';
    const RIGHT_SIDE = 'd';
    const VERTICAL_SIDES = self::LEFT_SIDE . self::RIGHT_SIDE;

    const ALL_SIDES = self::HORIZONTAL_SIDES . self::VERTICAL_SIDES;


    public function makeTest($sideType)
    {
        if($this->containsSide($sideType, self::TOP_SIDE)) {
            echo 'a';
        }

        if($this->containsSide($sideType, self::BOTTOM_SIDE)) {
            echo 'b';
        }

        if($this->containsSide($sideType, self::LEFT_SIDE)) {
            echo 'c';
        }

        if($this->containsSide($sideType, self::RIGHT_SIDE)) {
            echo 'd';
        }
    }

    private function containsSide($sideType, $singleSide)
    {
        return false !== strpos($sideType, $singleSide);
    }
}

$test = new Test();
$test->makeTest(Test::ALL_SIDES);

PhpFiddle

Answer 2

ИМХО, это один из случаев, когда вложенные ifs более читабельны.Это только один способ для этой ситуации, может быть не лучшим образом подходит для других ситуаций.

if ($sides==self::ALL_SIDES) {
    // do stuff, that (nearly) always should apply

    if ($sides==self::HORIZONATAL_SIDES) {
          // do stuff that applies to all horizontal sides
          if ($sides==self::TOP_SIDE) {
               // do top-stuff
          } elseif ($sides==self::BOTTOM_SIDE)  {
               // do other stuff
          }
    }
    // copy & adjust the same for vertical...
} else {
   // oops, an error!
}

Answer 3

Если логика всегда вызывает функцию, вы можете создать ассоциативный массив, который сопоставляет каждое из различных значений с соответствующей функцией.

$vertical_side_map = [
    self::LEFT_SIDE => 'c',
    self::RIGHT_SIDE => 'd'
];

foreach ($vertical_side_map as $key => $func) {
    if(in_array($sides, [self::ALL_SIDES, self::VERTICAL_SIDES, $key])) {
        $func();
    }
}


$horizontal_side_map = [
    self::TOP_SIDE => 'a',
    self::BOTTOM_SIDE => 'b',
];

foreach ($horizontal_side_map as $key => $func) {
    if(in_array($sides, [self::ALL_SIDES, self::HORIZONTAL_SIDES, $key])) {
        $func();
    }
}

Answer 4

Попробуйте это

$dos =($sides === self::TOP_SIDE)        ? ['a'] : (
      ($sides === self::BOTTOM_SIDE)     ?     ['b'] : (
      ($sides === self::LEFT_SIDE)       ?         ['c'] : (
      ($sides === self::RIGHT_SIDE)      ?             ['d'] : (
      ($sides === self::HORIZONTAL_SIDES)? ['a','b'] : (
      ($sides === self::VERTICAL_SIDES)  ?         ['c','d'] : (
      ($sides === self::ALL_SIDES)       ? ['a','b','c','d']))))));

foreach($dos as $do) $do();