CODEWITHSUNDEEP
c#struct
Max Yankov
Max Yankov

Reputation: 13297

Is it a bad practice to base logic on assumption that a type is a struct?

In my current project, I need to throw a lot of data around with a good understanding of what is a copy and what is a reference, in particular — implement a lot of deep cloning. Thankfully, a lot of my data is contained in simple structs, so I can just assign them or pass them as parameters and be sure that they aren't referencing each other.

However, I realized that the code that is doing that is based on assumption that these types are struct, and that if anyone is going to change their type to a class some time down the road, they may not notice errors right away, and when they do, it will be a hell to debug.

In general, I consider code which correctness is based on assumption that is not explicitly stated in the same file to be a smell, and especially when this assumption can be changed without immediately breaking the build. So, this situation really gives a bad smell for me.

Is what I'm doing really a bad practice? Have you experienced any problems that I described in real codebases, or am I imagining things? And if you have, what would be a good way to make things more explicit?

Here's an example of what I'm talking about (this is my actual code, with all names changed and other functionality stripped):

public class Example
{
    public readonly DataOfType;
    public readonly DataOfOtherType;

    public Example(Type _DataOfType, OtherType _DataOfOtherType)
    {
        DataOfType = _DataOfType;
        DataOfOtherType = _DataOfOtherType;
    }

    public Example DeepClone()
    {
        return new Example(DataOfType, DataOfOtherType);
    }
}

The DeepClone method is correct because Type is an enum and OtherType is a struct, but you would never know it from this code.

Upvotes: 2

Views: 121

Answers (3)

Guffa
Guffa

Reputation: 700472

Instead of thinking of data in terms of references and copies, I think that you should think about it in terms of mutability.

If you have an immutable type, then it doesn't matter if it is a struct or a class, you can treat it the same becase you know that it can't change. You can safely pass it around without caring about whether it's a reference or a copy, that becomes just an implementation detail.

Whenever you need to change a value, the immutability will force you to create a copy. That way you can never mess up a value in one place that is used in another place.

The String class is an example of how this is used. As strings are immutable you can pass them around as references without the risk of a string ever changing. All the methods in the String class that makes changes to a string will create a new string with the result.

Upvotes: 3

Patrick Hofman
Patrick Hofman

Reputation: 157018

If you make the struct immutable, which it should be in my opinion, you won't have issues with references being spread once the struct becomes a class.

Besides that, I think you have to help the 'maintainer' and 'user' of the struct to understand the implications.

You can easily do that with a remarks section in your Visual Studio code documentation. This will pop up on every one referencing the type.

I wouldn't make too much changes to the code to show the data type is a struct or do some type checking. This may only lead to confusion.

Upvotes: 1

Damien_The_Unbeliever
Damien_The_Unbeliever

Reputation: 239734

You could create a generic constrained to only allow structs, but uncreatable, and then create variables of that type at the top of each method where the assumption is required.

E.g.:

public class AssumeStruct<TStruct> where TStruct : struct
{
    private AssumeStruct(){}
}

And then:

public void MethodAssumingStructSemantics(S1 arg1)
{
   AssumeStruct<S1> t = null;
   ...
}

This should then cause a compiler error if the nature of the S1 type is changed later.

Upvotes: 3

Related Questions