CODEWITHSUNDEEP
c#.net
Mr. Boy
Mr. Boy

Reputation: 63728

Is there any way to prevent/detect automatic/implicit type conversion?

Consider these methods, each pulls bytes from an input buffer:

byte ReadByte(List<byte> data); //removes and returns 1 byte
UInt16 ReadUInt16(List<byte> data); //removes and returns 2 bytes as a short
UInt32 ReadUInt32(List<byte> data); //removes and returns 4 bytes as an int

Now I have a struct/class like:

class Thing
{
 public byte a{get;set;}
 public UInt16 b{get;set;}
 public UInt32 c{get;set;}
 public byte d{get;set;}

 public void Read(List<byte> data)
 {
  a = ReadByte(data);
  b = ReadUInt16(data);
  c = ReadUInt16(data); //BUG this is a 32bit data
  d = ReadByte(data);
 }
}

Because a short will automatically get promoted to an int, this code compiles and runs fine but has introduced the sort of bug that is hard to find - it has read 2 bytes fewer than it should for c and all subsequent values read will be wrong.

Are there any techniques that can be used to ensure that when c is of type UInt32, it will not accept a UInt16 or other legal type?

Ideally Thing would not be changed but if your solution requires it, that's ok.

Upvotes: 2

Views: 308

Answers (4)

KloppyToppy
KloppyToppy

Reputation: 264

You can create wrapper types and use them as return types for your methods as well as for the backing fields of your properties (which then need to be explicitly handled), like such:

class WrapperTypesPreventCast
{
    private MyByte _a;
    public byte a { get { return _a; } set { _a = value; } }
    private MyUInt16 _b;
    public UInt16 b { get { return _b; } set { _b = value; } }
    private MyUInt32 _c;
    public UInt32 c { get { return _c; } set { _c = value; } }

    public void Read(List<byte> data)
    {
        _a = ReadByte(data);
        _b = ReadUInt16(data);
        //_c = ReadUInt16(data);  // Would produce a compiler error.
        _c = ReadUInt32(data);    // Compiles
    }

    // Dummy implementations
    MyByte ReadByte(List<byte> data) => 0;
    MyUInt16 ReadUInt16(List<byte> data) => 0;
    MyUInt32 ReadUInt32(List<byte> data) => 0;
}

struct MyByte
{
    public byte Value { get; }

    public MyByte(byte value)
    {
        Value = value;
    }

    public static implicit operator byte(MyByte b) => b.Value;
    public static implicit operator MyByte(byte b) => new MyByte(b);
}

struct MyUInt16
{
    public UInt16 Value { get; }

    public MyUInt16(UInt16 value)
    {
        Value = value;
    }

    public static implicit operator UInt16(MyUInt16 b) => b.Value;
    public static implicit operator MyUInt16(UInt16 b) => new MyUInt16(b);
}

struct MyUInt32
{
    public UInt32 Value { get; }

    public MyUInt32(UInt32 value)
    {
        Value = value;
    }

    public static implicit operator UInt32(MyUInt32 b) => b.Value;
    public static implicit operator MyUInt32(UInt32 b) => new MyUInt32(b);
}

Errors are of course still possible, but due to the higher redundancy they are less likely. Furthermore, you can now easily create unit tests for the Read*() methods and verify that they indeed return the correct type (which would not have helped you in your original code, since the methods themselves are correct, just the usage isn't).

Note that the implicit conversions in the wrapper type are not strictly necessary, but increase the comfort of using them. However, you should restrict the accessibility to a minimum as otherwise this could lead to surprising and confusing conversions elsewhere in your code.

Upvotes: 0

user12031933
user12031933

Reputation:

The answer is: no for the methods you provide.

Such as C# and the CTS of .NET are designed, you can't avoid such coding mistake.

But that said, one thing you can do is to use a generic method like that:

static T Read<T>(this T instance, List<byte> data)
{
  switch ( instance )
  {
    case byte value:
      Console.WriteLine("Read byte");
      return default;
    case UInt16 value:
      Console.WriteLine("Read UInt16");
      return default;
    case UInt32 value:
      Console.WriteLine("Read Uint32");
      return default;
    default:
      string message = $"Type not supported for Read<T> (only byte or UInt16/32): "
                     + $"{typeof(T).Name}";
      throw new ArgumentException(message);
  }
}

class Thing
{
  public byte a { get; set; }
  public UInt16 b { get; set; }
  public UInt32 c { get; set; }
  public byte d { get; set; }
  public void Read(List<byte> data)
  {
    a = a.Read(data);
    b = b.Read(data);
    c = c.Read(data);
    d = d.Read(data);
  }
}

This does not provide an absolute way to not do coding error but to do less because we can easily see a match with the variable we manipulate like when using strings methods to modify the same string.

Test

var list = new List<byte> { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 };

var instance = new Thing();

instance.Read(list);

Output

Read byte
Read UInt16
Read Uint32
Read byte

Upvotes: 3

itsme86
itsme86

Reputation: 19496

It's not the most elegant thing, but you could make each of your Read*() methods generic and do a type check that looks kind of redundant. Unfortunately, you can't use a property as an out or ref parameter, otherwise this could be a lot simpler:

UInt16 ReadUInt16<T>(List<byte> data, T _)
{
    if (typeof(T) != typeof(UInt16))
        // Throw exception
    
    // Perform normal process and return value.
}

Then (this is the redundant-looking part) you can utilize it like:

c = ReadUInt16(data, c);

Upvotes: 1

Daniel A. White
Daniel A. White

Reputation: 190943

The best way is to write unit tests validating that the inputs match the expected outputs. You could also do reflection to do the logic automatically or do things with a code generator.

There isn't anything at build time to help you, unless you wanted to write a Roslyn analyzer.

Upvotes: 3

Related Questions