Class + Struct is a good coding style?

Question

I am reading existing code. I noticed that there are many data object files which have a struct and a class together to define a data object. Like the following one: do you think it is a good style?

In ONE file:

[StructLayout(LayoutKind.Sequential, Pack = 1)]
    public struct LaneDataStruct
    {
        public ushort red;
        public ushort yellow;
        public ushort green;
        public ushort blue;
        public ushort orange;
    }

public class LaneData
    {
        private LaneDataStruct laneDataStruct;
    public LaneData(ushort red, ushort yellow, ushort green, ushort blue, ushort orange)
    {
        this.laneDataStruct.red = red;
        this.laneDataStruct.yellow = yellow;
        this.laneDataStruct.green = green;
        this.laneDataStruct.blue = blue;
        this.laneDataStruct.orange = orange;
    }

    public LaneData(ushort[] values)
    {
        this.laneDataStruct.red = values[0];
        this.laneDataStruct.yellow = values[1];
        this.laneDataStruct.green = values[2];
        this.laneDataStruct.blue = values[3];
        this.laneDataStruct.orange = values[4];
    }

    public LaneData(LaneDataStruct laneDataStruct)
    {
        this.laneDataStruct = laneDataStruct;
    }

    public LaneDataStruct getLaneDataStruct()
    {
        return this.laneDataStruct;
    }

    public string toString()
    {
        StringBuilder stringBuilder = new StringBuilder();
        stringBuilder.Append("LaneData.red=" + this.getLaneDataStruct().red + "\n");
        stringBuilder.Append("LaneData.yellow=" + this.getLaneDataStruct().yellow + "\n");
        stringBuilder.Append("LaneData.green=" + this.getLaneDataStruct().green + "\n");
        stringBuilder.Append("LaneData.blue=" + this.getLaneDataStruct().blue + "\n");
        stringBuilder.Append("LaneData.orange=" + this.getLaneDataStruct().orange);

        return stringBuilder.ToString();
    }
}

While you're not referring to this, I think it *is* bad style to add a public `toString()` method when you should actually override `ToString()`, and also using a string builder, but then concatenating strings instead of using `AppendFormat()`... — OregonGhost, Feb 22 '10 at 14:23

score 3 · Answer 1 · answered Feb 22 '10 at 14:24

3

The fact that it's a mutable struct with public fields is bad style to start with.

I can't say I've seen it used, and I wouldn't want to really.

answered Feb 22 '10 at 14:24

Jon Skeet

1,421,763
867
9,128
9,194

4

I use mutable structs for low-level communication with bus systems or embedded hardware all the time, because it's the most exact translation from typical embedded header files (with all that MarshalAs and StructLayout stuff). I guess you *could* use classes for that, but I don't see the point. For code in general, of course, I agree with you, but I think there *is* an area where they can be used. – OregonGhost Feb 22 '10 at 14:29

score 0 · Answer 2 · answered Feb 22 '10 at 14:22

Structs in C# are POD types -- with slightly different semantics. MSDN says this:

The struct type is suitable for representing lightweight objects such as Point, Rectangle, and Color. Although it is possible to represent a point as a class, a struct is more efficient in some scenarios.

Even if this distinction wasn't there (C++) it is good practise to differentiate POD types using a seperate keyword.

score 0 · Accepted Answer · edited May 23 '17 at 12:11

I would only call this a good practice in an exotic P/Invoke scenario, and even then it's questionable.

First, the struct is poorly designed since it is mutable. See this question for more information.

The class looks like it was written to address the fact that in C#, there is no deterministic way to initialize a structure. You can't give a struct a default constructor, and you can't cause a non-default constructor to be called. So it looks like the class wraps the structure and gives a guarantee that the structure will be initialized to some state. However, it does no validation so I strongly doubt there is any value to it at all.

This would only be "useful" if LaneDataStruct represents a structure that is used for P/Invoke (the LayoutKind attribute is a hint this may be true) and that can't have certain values. It would still be preferable to give it properties that perform validation and make the fields private. Usually, P/Invoke code is written in a separate layer where it's considered acceptable to have warts like "make sure to initialize this struct after creating one".

P/Invoke is the name of the mechanism .NET uses to call functions in Windows DLLs. — OwenP, Feb 22 '10 at 19:17

score 0 · Answer 4 · answered Feb 22 '10 at 14:38

The only reason I would use a Struct in this manner, especially with a StructLayout Attribute is if I were doing PInvoke to a 3rd party or legacy C DLL, where I had to pass this structure to be marshalled, or if I were doing some sort of serial communications protocol over the wire where I wanted my structure to exactly match the packet I was sending/receiving.

Class + Struct is a good coding style?

4 Answers4