Reducing function size

159 Views Asked by At

Maybe this is a dummy question but as I am not a C# expert I think this function could be written better with using fewer lines of code.

Here it is:

    public void chgnav(string wt, string nav)
    {
        if (wt == "enable")
        {
            if (nav == "prev")
            {
                pictureBox7.Visible = true;
                pictureBox9.Visible = false;
            }
            else
            {
                pictureBox8.Visible = true;
                pictureBox10.Visible = false;
            }
        }
        else
        {
            if (nav == "prev")
            {
                pictureBox7.Visible = false;
                pictureBox9.Visible = true;
            }
            else
            {
                pictureBox8.Visible = false;
                pictureBox10.Visible = true;
            }

        }
    }

Edit: Thanks to everyone, upvotes from me. I got what I was looking for.

4

There are 4 best solutions below

0
agent-j On BEST ANSWER
public void chgnav(string wt, string nav)
{
   bool wtEnabled = wt == "enable";
   if (nav == "prev")
   {
      pictureBox7.Visible = wtEnabled;
      pictureBox9.Visible = !wtEnabled;
   }
   else
   {
      pictureBox8.Visible = !wtEnabled;
      pictureBox10.Visible = wtEnabled;
   }
}

Edit: fixed

0
Mark On

I think that the intent of the method is clear, and the implementation is clean. It may be a few lines longer than you'd like, but obfuscation for the sake of a few lines of code is a net loss in my book.

I'd keep it the way it is.

0
Peter O. On

This may work:

public void chgnav(string wt, string nav)
{
        if (nav == "prev")
        {
            pictureBox7.Visible = (wt=="enable");
            pictureBox9.Visible = (wt!="enable");
        }
        else
        {
            pictureBox8.Visible = (wt=="enable");
            pictureBox10.Visible = (wt!="enable");
        }
}

Or even:

public void chgnav(string wt, string nav)
{
    (nav=="prev" ? pictureBox7 : pictureBox8).Visible = (wt=="enable");
    (nav=="prev" ? pictureBox9 : pictureBox10).Visible = (wt!="enable");
}
0
Mark Robbins On

That works fine for the limited scope you have, but suppose it is 10 times bigger and has more inputs.

What you want is a 'truth table' setup.

You have a dictionary that takes a key corresponding to your params.

given params 'isBlue' and 'isBig', your dictionary keys would be

'ff' //not blue or big

'tf' //blue but not big

etc.

The dictionary keys lead you to a value that is a map of the objects, property names and their values.

So your map could be a list of Tuples. Where Tuple<object, string, object>, or simply a struct of same.

Then your code would do some ifs to compose the key, get the List of Tuples or structs from the dictionary, run down the list to do what is proper for each object.