S109 FP: Array indices and declarations

Rules and Languages Report False-positive / False-negative...
1

Originally from a comment on a GitHub issue.

The user sharbhajan shared his view that S109 should not raise on array indices and declarations. I’m moving the conversation here.

Examples:

byte[] bytes = new byte[10];
var time = DateTime.Now;
bytes[3] = (byte)time.Year;
bytes[2] = (byte)time.Month;
bytes[1] = (byte)time.Day;
bytes[0] = (byte)time.Hour;
bytes[5] = (byte)time.Minute;
bytes[4] = (byte)time.Second;

//gif signatures
AddSignatures(new byte[] { 0x47, 0x49, 0x46, 0x38 });
1 Like
3

I would argue that this

AddSignatures(new byte[] { 0x47, 0x49, 0x46, 0x38 });

can easily be solved differently:

private static readonly byte[] GifHeader = [0x47, 0x49, 0x46, 0x38];

// ...

AddSignatures(GifHeader);

And I would argue that this is exactly why S109 exists.

For this example, I think it’s different.

byte[] bytes = new byte[10];
var time = DateTime.Now;
bytes[3] = (byte)time.Year;
bytes[2] = (byte)time.Month;
bytes[1] = (byte)time.Day;
bytes[0] = (byte)time.Hour;
bytes[5] = (byte)time.Minute;
bytes[4] = (byte)time.Second;

When I read this, my head is spinning: why is this bizarre ordering in place? By using constants for those indexes, I do not think the story of the code will become clearer.

var time = DateTime.Now;

var bytes = new byte[10]
{
    (byte)time.Hour,
    (byte)time.Day,
    (byte)time.Month,
    (byte)time.Year,
    (byte)time.Second,
    (byte)time.Minute,
    // ...
}

This is also not clear and I already feel the urgency to start using block comments like /* 0 */ to make the index better understood. You could argue that this code is bad in so many ways that no rule could safe the day. But lets for now assume that the order of the bytes is not in control of the person who had to write and/or maintain the code, than maybe just suppressing the issue here is fine?!

1 Like