Is there any way to simplify this amount of conditions?

Asked

Viewed 116 times

-2

I have the following code:

        foreach (var lote in collection.ToList())
        {
            count++;

            ushort[] dadosBalanca = new ushort[9];

            if (lote.AlvoPasso > ushort.MaxValue)
            {
                var subtracao = (lote.AlvoPasso - 65536);
                dadosBalanca[3] = 1;

                if (lote.AlvoPasso > (ushort.MaxValue * 2))
                {
                    subtracao -= 65536;
                    dadosBalanca[3] = 2; //[OUT37]

                    if (lote.AlvoPasso > (ushort.MaxValue * 3))
                    {
                        subtracao -= 65536;
                        dadosBalanca[3] = 3; //[OUT37]

                        if (lote.AlvoPasso > (ushort.MaxValue * 4))
                        {
                            subtracao -= 65536;
                            dadosBalanca[3] = 4; //[OUT37]
                        }
                    }
                }

                dadosBalanca[4] = (ushort)subtracao; //[OUT38]
            }
            else
            {
                dadosBalanca[3] = 0; //[OUT37]
                dadosBalanca[4] = (ushort)lote.AlvoPasso; //[OUT38]
            }

            //...
        }

This code already works, I would like to suggest improvement.

c# .net code-review

  • I see no need to "dry" the code, the same is with the correct syntax. What is the need that you are having?

  • Do you have more code inside the foreach? This is relevant (not the code itself, but the information) because you may want to use a continue to decrease the level of indentation.

  • Although the code works, from the comments in the answers it is clear that the necessary rules have not been defined, so I closed. Can be edited to put the missing parameters and reopened.

2 answers

3

Yes, it does, quite:

var deslocamento = lote.AlvoPasso / ushort.MaxValue;
dadosBalanca[3] = deslocamento;
dadosBalanca[4] = lote.AlvoPasso - ushort.MaxValue * deslocamento;

I put in the Github for future reference.

I’m assuming there’s no way the displacement is more than 4, since the current code ignores this. If you can, you’ll get the original and mine wrong, you’d need to treat, but you’d need to see the criteria.

  • in this case the displacement may have more than 4, just not put more, not to be extended the question, so I want to improve, the criterion is always multiply +1, in this case follow the logic of the if, can understand?

2


It doesn’t have much to garnish. You can replace those if’s chained by a for.

BTW, the three if’s being chained doesn’t make much sense because they write in the same place, ie, one overwrites the other. Anyway, I kept the original behavior in for.

The code can go like this:

foreach (var lote in collection.ToList())
{
    count++;
    ushort[] dadosBalanca = new ushort[9];

    if (lote.AlvoPasso <= ushort.MaxValue) // Note que isso foi invertido
    {
        dadosBalanca[3] = 0; //[OUT37]
        dadosBalanca[4] = (ushort)lote.AlvoPasso; //[OUT38]
        continue;        
    }

    var subtracao = (lote.AlvoPasso - 65536);
    dadosBalanca[3] = 1;

    for(ushort i = 2; i <= 3; i++)
    {
        if (lote.AlvoPasso > (ushort.MaxValue * i))
        {
            subtracao -= 65536;
            dadosBalanca[3] = i; //[OUT3X]    
        }    
    }        

    dadosBalanca[4] = (ushort)subtracao; //[OUT38]    
}

  • I edited your question but it was rejected, so I want to suggest to you to edit, inside the for, change the var to ushort or cast before assigning in the position dataBalanca[3] if not from the error, since var i=1 assumes that i is integer and not ushort, and change the initial value of the for to 2, because the first subtraction you already made out of the for. Got it?

Browser other questions tagged c# .net code-review

You are not signed in. Login or sign up in order to post.