-2

I have a Button an its function. When the user presses a Button, a Line of Textboxes change their Backcolor to grey. When the user press the button again, the Line change back to white. But my solution for that is way to complicated and i think there is a more comfortable way to do this. Here is my Code:

 private void btnTitelRow2_Click(object sender, EventArgs e)
    {
        bool isTitel = true;
        bool checkStatus = CheckTitelStatus(row: 1);
        if (checkStatus == true)
        {
            isTitel = false;
        }
        fncLOITitelrow(row: 1, isTitel: isTitel);
    }



    public bool CheckTitelStatus(int row)
    {
        bool labelStatus = false;
        Color dimgray = System.Drawing.Color.LightGray;
        Color white = System.Drawing.Color.White;

        Label[] Titelbl = getLOILabelTitel();
        if (Titelbl[row].BackColor == white)
        {
            labelStatus = false;
            return labelStatus;
        }
        if (Titelbl[row].BackColor == dimgray)
        {
            labelStatus = true;
            return labelStatus;
        }
        else
        { return labelStatus; }

the function LOITitelrow is the one that changes the line of textboxes color. i think waht i need is a simpel way to give me a true when the button is clicked for the first time, and a false when it is pressed the second time. 3rd time true and so on...

any ideas?

1 Answers1

2
private void btnTitelRow2_Click(object sender, EventArgs e)
{
    const int row = 1;
    fncLOITitelrow(row, CheckTitelStatus(row));
}

public bool CheckTitelStatus(int row)
{
    Label[] Titelbl = getLOILabelTitel();
    return Titelbl[row].BackColor == System.Drawing.Color.LightGray;
}

You're writing a lot of redundant code, such as comparing a boolean to true. The result of CheckTitelStatus returns a boolean, and an if statement expects a boolean value, therefore you can just use the return value of CheckTitelStatus. Also, in your CheckTitelStatus you can just check if the BackColor is currently LightGray, instead of checking for each color.

Amit Beckenstein
  • 1,220
  • 12
  • 20
  • 2
    You could correct `Titel` to `Title` so it doesn't hurt everyone's eyes – Camilo Terevinto Jan 14 '18 at 00:09
  • @CamiloTerevinto haha I was thinking of doing so but I didn't want to confuse him in case he copies and pastes this answer. – Amit Beckenstein Jan 14 '18 at 00:10
  • 2
    @Camilo Terevinto `Titel` is `Title` in German, and to efficiently change it, he should go to the OP's place and change the name of the Button Control, too. Too much unpaid work, I think :). – Jimi Jan 14 '18 at 00:21
  • @Jimi Not sure what German has to do with anything, as the rest of the code is in English... – Camilo Terevinto Jan 14 '18 at 00:41
  • @Camilo Terevinto The OP is from Switzerland. He probably mixed up languages. And the name of the button is `btnTitelRow2`. – Jimi Jan 14 '18 at 01:11
  • @Amit B. thank you very much for the help! i have one further question: wy did you declare a constant? is it so you can better see that this number allways have to be 1? – Jan Turnherr Jan 14 '18 at 01:23
  • Writing the Const into the function protects you from accidently using it anywhere else. It is entirely a scope thing, with no actually effect on runtime performance or anything (as those values are hardcoded at compile time anyway). See also: https://stackoverflow.com/questions/6373072/defining-local-variable-const-vs-class-const – Christopher Jan 14 '18 at 02:02
  • I used const because it is less hard-coded this way. It, too, is for declaring that the number will always be 1, yes. – Amit Beckenstein Jan 14 '18 at 09:43