Search code examples
c#oopconstantsimmutability

C# object's returning internal components


Goal

A very recurrent design I would like to implement in C# is following: A class, that owns multiple instances of another class.

For clarity let's pick an example, let's say a "car" that owns four "wheels".

Here are the constraints I want to fulfill:

  • I want the car to show its wheels, not to hide them. => There should be in car class a thing like

    public Wheel GetWheel(Left, Rear)
    
  • I want the car to be able to update its wheels in some way. For example, there could in car class a method like:

    public void ChangeTires(TireType)
    
  • I don't want anybody else to be able to update the wheels. For example, it should not be possible to run something like:

    aCar.GetWheel(Left, Rear).SetTire(someTireType); // Does not compile
    

This is a very very recurring scheme: a Network has Nodes, a Chair has Back, a User has Credentials, a Socket has IPAddress, a BankAccount has an Owner...

I read forums and asked co-workers how they do that. Multiple answers were given to me. Here they are.

My questions are:

  • Is there another way to achieve this goal that is not listed above?
  • Is there a general consensus on the "good" way to do?
  • Is there a documented reflection on this topic somewhere? Brilliant people that made C# or use it extensively probably solved this topic a long time ago.

Solution attempt 1: using internal

Some people are advising to limit the visibility of the SetTire method to the current assembly:

class Wheel
{
   internal void setTire(TireType someTireType);
}

Only the "files in the same assembly" as Wheel are able to update it (to change Tires).

But it implies that Car and Wheel are defined in the same assembly, which by transitivity makes everything to be defined in the same assembly.

Additionally, it's not enough to limit it to the same assembly, I want to limit it to the Car.

Solution attempt 2: return a copy

Some people are advising that the GetWheel method should return a copy:

class Car
{
   public Wheel LeftRearWheel;

   public Wheel GetWheel(LeftOrRight, FrontOrRear)
   {
       return LeftRearWheel.DeepCopy();
   }
}

With this solution, there's nothing telling the client-code that the wheel is a copy. Which means, each time the client coder writes a get, he doesn't know if he may or may not update the returned object.

Additionally, it may be a performance concern to deep-copy everything you use in your application.

Said differently: the following code compiles, but does nothing:

aCar.GetWheel(Left, Rear).SetTire(someTireType); // Compiles but does nothing

Solution attempt 3: create an immutable wheel

Some people are advising to do this :

public class ReadonlyWheel
{
    public Tire getTire();
}

internal class Wheel : ReadonlyWheel
{
    internal void setTire(Tire);
}

public class Car
{
    public Wheel LeftRearWheel;

    public ReadonlyWheel GetWheel(LeftOrRight, FrontOrRear)
    {
        return LeftRearWheel;
    }
}

A variant is to declare an interface

public interface IWheel
{
    TireType getTire();
}

internal class Wheel: IWheel
{
    public TireType getTire();
    internal void setTire(Tire);
}

public class Car
{
    public Wheel LeftRearWheel;

    public IWheel(LeftOrRight, FrontOrRear)
    {
        return LeftRearWheel;
    }
}

This solution does achieve the goal, but implies a lot of code duplication and additional complexity. As this is a very recurrent pattern, I would like to avoid code duplication.

[Edit]

By request, I'm adding details on the drawbacks of this solution:

This solution implies, that for each class that is owned by another (that is : most of the classes), we need to create an interface. It implies declaring every non-modifying public method twice: once in the interface, once in the class.

[/Edit]

Solution attempt 4: don't care

Some people are claiming that the question itself is outside of the "C# way of thinking".

According to some, ensuring that my users aren't doing "stupid" things with my model is none of my concern.

A variant is: "if it happens, that means your model is bad". The Wheel should either be fully private or fully public.

I fail to bring myself to think this way. Maybe someone could point to any documentation that explains how the "C# way of thinking" achieve this kind of modelisation.

Solution attempt 5: using ReadOnlyCollection

If the object I want to show is a container, there is a class named ReadOnlyCollection that, as I understand it, combine solution attempt 2 and 3: it makes a copy and embeds it in a readonly interface:

class Car
{
    private List<Wheel> myWheels;

    public ReadOnlyCollection<Wheel> GetWheels()
    {
        return myWheels.AsReadOnly();
    }
}

It achieves a part of the goal, which is warning the user he should not try to update the collection. But it fails to prevent the update. Said differently, the following code compiles and does nothing.

aCar.GetWheels()[0].SetTire(someTireType); // Compile but does nothing

Additionally, it does not apply to non-collections.


Solution

  • I'm not sure that I understand the problem, but I'll try to summarise what I do understand:

    • Car should hold some Wheels
    • Car should have a ChangeTires method
    • Callers should not be able to call ChangeTires on Wheel - only via Car.

    If so, one way you can do that is to separate the public interface from the implementation.

    First, define the public API you'd like to expose to client code:

    public interface IWheel
    {
        // Put some members here that DON'T include changing tires...
        // Current pressure, size, etc.
    }
    

    Now create the Car class and make the IWheel implementation a private nested class inside of Car:

    public sealed class Car
    {
        private readonly Wheel leftRearWheel = new Wheel();
    
        public IWheel LeftRearWheel { get { return leftRearWheel; } }
        // Other wheels here...
    
        public void ChangeTires(TireType tireType)
        {
            leftRearWheel.ChangeTire(tireType);
            // Other wheels here...
        }
    
        private sealed class Wheel : IWheel
        {
            public void ChangeTire(TireType tireType)
            {
                // Change the tire
            }
        }
    }
    

    While the nested Wheel class' ChangeTire method is marked public, it's only visible to the Car class, since the Wheel class is a private class.

    Thus, Car can call ChangeTire on all Wheel objects, but no other classes can.