Dynamic menu depending on a set mode

I am creating a dynamic menu whose items appear depending on a set 'mode' (that is passed via AJAX). Off this it creates the menu, disabling and hiding icons that are not associated with that mode.

The problem is, there are a lot of if conditions in my implementation. Can anyone show me a cleaner way of doing what I am trying to achieve?

public function gridMenu()
  {
    $mode = Validate::sanitize($_POST['mode']);

    $modes = array(
        'products' => array('edit', 'delete', 'archive')
    );

    $output = '';
    for($i = 1; $i ';
        } else {
          $output .= ' hex-disabled';
        }
      }
      if($i == 2)
      {
        if(in_array('zzz', $modes[$mode]))
        {
          $output .= ' hex-selectable';
        } else {
          $output .= ' hex-disabled';
        }
      }
      if($i == 3)
      {
        if(in_array('delete', $modes[$mode]))
        {
          $output .= ' hex-selectable';
          $img = '';
        } else {
          $output .= ' hex-disabled';
        }
      }
      if($i == 4)
      {
        if(in_array('xxx', $modes[$mode]))
        {
          $output .= ' hex-selectable';
        } else {
          $output .= ' hex-disabled';
        }
      }
      if($i == 5)
      {
        if(in_array('archive', $modes[$mode]))
        {
          $output .= ' hex-selectable';
          $img = '';
        } else {
          $output .= ' hex-disabled';
        }
      }
      if($i == 6)
      {
        if(in_array('zzz', $modes[$mode]))
        {
          $output .= ' hex-selectable';
        } else {
          $output .= ' hex-disabled';
        }
      }

      $output .= '">';
      $output .= $img;
      $output .= '';
    }
    $output .= '';
    $output .= '';
    echo $output;
  }

Your code has multiple bad practices.

There is no speration of concern. Your code is handling display, logic and controlling. All in onde function.

If I look at the function, it tells me it needs nothing (no arguments need to be passed in). So the only thing it should be using are variables that are accessible inside the object scope. BUT it turns out you are using a 'Validate' class and variables from the global scope ($_POST). I only know this because I read the entire code. No really nice towards other developers or the future you demaning that one should read the entire code before knowing how to use it and what is going on. And then ofcourse the biggest problem. al those if's

So, some problems and lets fix those problems.

You sanitize a variable that is handed in by the client ($_POST). You then create an array of possible 'modes' out of the blue. And after that you construct some html with images and a lot of business logic in it. Your function really has to be all knowing!

So, let's sepperate the concerns.

the $mode is clearly something the function needs to do it's magic. So this sould be a paramter that is passed in:

public function gridMenu(String $mode);

Note that the $mode needs to be a string. (obviously)

Another thing this function absolutely needs is the $modes variable. But, this variable doesn't change with every call of the gridMenu() function. It is the same, and should be, during runtime. But it could change over time or even change depending on what user is logged in. So we place this in the constructor of the class:

class Grid {

  private $modes;

  public function __construt($modes) {
    $this->$modes = $modes;
  }

  public function gridMenu(String $mode) { }
}

Now, gridMenu doesnt really say much. I would have to guess what it does: save a menu? does it create one? does it return one? So lets change that name to something more understandable:

class Grid {

  private $modes;

  public function __construt($modes) {
    $this->$modes = $modes;
  }

  public function renderMenu(String $mode) { }
}

Other possibilities are echoMenu, printMenu... Note that i left the word grid out because we are allready in the grid class. Obviously we wouldn't print a non-grid menu out here...

Good, so now we can remove that Validate::sanitize to the outside of the code and that declaration of $modes is no longer hard-coded into the class. This gives us a lot more flexibility,...

Now let's look at what our function does. It creates html. wheut? So it doesn't provide functionality but simply prints some html? So it's is actually a template disguised as a function... the sneaky bastard. We really have to keep an eye on those templates mate. They tend to creep into functions all the time all over the code.

This makes them harder to maintain and makes you write tons of Repeat-Yourself code. For instance if we would no want it to return the data in XML, or in a list instead of divs. Or even worse, have the delete button placed somewhere else, or or.... OOh boy, beter start all over again.

Needless to say, we are doing something really wrong here and we have read all this text for nothing :( Well atleast you know about sepration of concern. And I give you an oher link: SOLID

But what has this todo with you question? Ah it's very easy. What you actually need is a template that handles the display of of those '.hex' divs (what is a hex? and why is action.delete/archive a hex?). You also need some business logic that calculates what 'actions' you can do. and then we need some controller to bring it all together.

Your template would look something like this:

  hasAction('edit') ? 'enabled' : 'disabled' ; ?>">
    hasAction('edit') ? '': ''; ?>
  

  
    //zzz
  

  hasAction('delete') ? 'enabled' : 'disabled' ; ?>">
    hasAction('edit') ? '': ''; ?>
  

  
    //xxx
  

  hasAction('archive') ? 'enabled' : 'disabled' ; ?>">
    hasAction('edit') ? '': ''; ?>
  

  
    //zzz
  

Your business logic 'Mode' and 'ModeFactory' class would look something like this:

class ModeFactory {

  private $modes;

  public function __construt(array $modes) {
    $this->$modes = $modes;
  }

  public function getMode(string $mode) {
    if ( !isset($this->modes[$mode]) ) {
      throw new ModeNotFoundException();
    }
    return new Mode($this->modes[$mode]);
  }
}

class Mode {
  private $actions = $actions;
  public function __construct(array $actions) {
    $this->actions = $actions;
  }
  public function hasAction(string $action) {
    return in_array($this->actions[$action]);
  }
}

class ModeNotFoundException extends Exception {}

NOTE: I used the Factory pattern here, is this the best? not, but it is a pattern that suited my needs and I feld like using it.

Then to bring it all togeter: the controller:

```
include 'Validate.php'; #the Validate class
include 'Mode.php'; #Modefactory and Mode

//note that now only the controller needs to know
// how t

StackExchange Code Review Q#29064, answer score: 2