views:

794

answers:

9

I have a class Bar with a private field containing the reference type Foo. I would like to expose Foo in a public property, but I do not want the consumers of the property to be able to alter Foo... It should however be alterable internally by Bar, i.e. I can't make the field readonly.

So what I would like is:

      private _Foo;

      public Foo 
      { 
         get { return readonly _Foo; } 
      }

...which is of course not valid. I could just return a clone of Foo (assumming that it is IClonable), but this is not obvious to the consumer. Should I change the name of the property to FooCopy?? Should it be a GetCopyOfFoo method instead? What would you consider best practice? Thanks!

+6  A: 

It sounds like you're after the equivalent of "const" from C++. This doesn't exist in C#. There's no way of indicating that consumers can't modify the properties of an object, but something else can (assuming the mutating members are public, of course).

You could return a clone of the Foo as suggested, or possibly a view onto the Foo, as ReadOnlyCollection does for collections. Of course if you could make Foo an immutable type, that would make life simpler...

Note that there's a big difference between making the field readonly and making the object itself immutable.

Currently, the type itself could change things in both ways. It could do:

_Foo = new Foo(...);

or

_Foo.SomeProperty = newValue;

If it only needs to be able to do the second, the field could be readonly but you still have the problem of people fetching the property being able to mutate the object. If it only needs to do the first, and actually Foo is either already immutable or could be made immutable, you can just provide a property which only has the "getter" and you'll be fine.

It's very important that you understand the difference between changing the value of the field (to make it refer to a different instance) and changing the contents of the object that the field refers to.

Jon Skeet
What is a view in this context? An interface or wrapper?
Joel in Gö
...and absolutely agree on the value/contents of reference distinction. Unfortunately I need to do both.
Joel in Gö
See my comment for what I think he means by view.
RossFabricant
@jon: Just curious Jon, why can't we have a private set property here ? 'public Foo { get { return _Foo; } private set { _Foo = value;} }'or am I missing something here ?
ram
@Ram: I don't see what that would accomplish. The idea is to expose a read-only view of Foo - not just the variable holding it, but the object itself.
Jon Skeet
+2  A: 

Making a copy, a ReadOnlyCollection, or having Foo be immutable are usually the three best routes, as you've already speculated.

I sometimes favor methods instead of properties whenever I'm doing anything more significant than simply returning the underlying field, depending on how much work is involved. Methods imply that something is going on and raise more of a flag for consumers when they're using your API.

John Feminella
+1  A: 

"Cloning" the Foo objects you receive and give back out is a normal practice called defensive copying. Unless there is some unseen side-effect to cloning that will be visible to the user, there is absolutely no reason to NOT do this. It is often the only way to protect your classes' internal private data, especially in C# or Java, where the C++ idea of const is not available. (IE, it must be done in order to properly create truly immutable objects in these two languages.)

Just to clarify, possible side effects would be things like your user (reasonably) expecting that the original object be returned, or some resource being held by Foo that will not be cloned correctly. (In which case, what is it doing implementing IClonable?!)

jdmichal
+3  A: 

Unfortunately, there's no easy way around this in C# at the moment. You could extract the "read only part" of Foo in an interface and let your property return that instead of Foo.

Brian Rasmussen
Less than ideal, because the interface could still be cast to a Foo.
RossFabricant
I agree - but you're at least revealing your intentions to the caller.
Brian Rasmussen
@rossfabricant: Decide whether you want to protect against Murphy or Machiavelli. Downcasting to Foo is Machiavellan. You can't design to protect against ill will, only against mistakes.
Pontus Gagge
Good point, and with reflection available to the caller, there's very little you can do against Machiavelli anyway. But in any case a const modifier would be better.
Brian Rasmussen
@pontus gagge-Awesome point. Wish I could upvote your comment more.
John MacIntyre
A: 

To clarify Jon Skeet's comment you can make a view, that is an immutable wrapper class for the mutable Foo. Here's an example:

class Foo{
 public string A{get; set;}
 public string B{get; set;}
 //...
}

class ReadOnlyFoo{
  Foo foo; 
  public string A { get { return foo.A; }}
  public string B { get { return foo.B; }}
}
RossFabricant
This only works if the properties of Foo are value types. If Foo has a ref type property foo.Obj, then the caller can still alter the object at will (although not set it to a new object, see John Skeet's remarks above). So it is not really immutable.
Joel in Gö
You could have ReadOnlyFoo have a property that returns a ReadOnlyBar.
RossFabricant
...and so on ad infinitum... Seems like an awful lot of overhead.
Joel in Gö
@Joel: That's what you get for having a deeply mutable type, basically. @rossfabricant: Yes, that's exactly what I meant, thanks :)
Jon Skeet
+1  A: 

If you don't want anyone to mess with your state...don't expose it! As others have said, if something needs to view your internal state, provide an immutable representation of it. Alternatively, get clients to tell you to do something (Google for "tell don't ask"), instead of doing it themselves.

Jim Arnold
A: 

You can actually reproduce the behaviour of C++ const in C# - you just have to do it manually.

Whatever Foo is, the only way the caller can modify its state is by calling methods on it or setting properties.

For example, Foo is of type FooClass:

class FooClass
{
    public void MutateMyStateYouBadBoy() { ... }

    public string Message
    {
        get { ... }
        set { ... }
    }
}

So in your case, you're happy for them to get the Message property, but not set it, and you're definitely not happy about them calling that method.

So define an interface describing what they're allowed to do:

interface IFooConst
{
    public string Message
    {
        get { ... }
    }
}

We've left out the mutating method and only left in the getter on the property.

Then add that interface to the base list of FooClass.

Now in your class with the Foo property, you have a field:

private FooClass _foo;

And a property getter:

public IFooConst Foo
{
    get { return _foo; }
}

This basically reproduces by hand precisely what the C++ const keyword would do automatically. In psuedo-C++ terms, a reference of type const Foo & is like an automatically generated type that only includes those members of Foo that were marked as const members. Translating this into some theoretical future version of C#, you'd declare FooClass like this:

class FooClass
{
    public void MutateMyStateYouBadBoy() { ... }

    public string Message
    {
        get const { ... }
        set { ... }
    }
}

Really all I've done is merged the information in IFooConst back into FooClass, by tagging the one safe member with a new const keyword. So in a way, adding a const keyword wouldn't add much to the language besides a formal approach to this pattern.

Then if you had a const reference to a FooClass object:

const FooClass f = GetMeAFooClass();

You would only be able to call the const members on f.

Note that if the FooClass definition is public, the caller could cast an IFooConst into a FooClass. But they can do that in C++ too - it's called "casting away const" and involves a special operator called const_cast<T>(const T &).

There's also the issue of interfaces not being very easy to evolve between versions of your product. If a third party may implement an interface you define (which they are free to do if they can see it), then you can't add new methods to it in future versions without requiring others to recompile their code. But that's only a problem if you are writing an extensible library for others to build on. Maybe a built-in const feature would solve this problem.

Daniel Earwicker
A: 

I know we're talking C# here...but you can do this in .NET with C++/CLI:

namespace Test
{
    public ref class Foo
    {
    public:
     property int Int;
    };
}

public ref class Bar
{
    Test::Foo^ _foo;
public:
    property const Test::Foo^ Foo
    {
     const Test::Foo^ get() { return _foo; }
    }
};

int main(array<System::String ^> ^args)
{
    Bar^ bar = gcnew Bar();
    bar->Foo->Int = 314;

    return 0;
}

The compiler will then complain: error C2662: 'Test::Foo::Int::set' : cannot convert 'this' pointer from 'const Test::Foo' to 'Test::Foo %'.

Reflector shows that const is done with a modopt

public class Bar
{
    private Foo _foo;
    public Foo modopt(IsConst) Foo
    {
        get { return this._foo; }
    }
}

which C# (obviously) doesn't pay any attention to as this code compiles:

 static void Main(string[] args)
 {
  global::Bar bar = new Bar();
  bar.Foo.Int = 314;
 }

I supose that perhaps leaves the door open a bit for C# supporting in some way modopt(IsConst) someday.

Dan
A: 

I'd create a readonly adapter for Foo.