7

In C# you can create getter/setters in a simpler way than other languages:

public int FooBar { get; set; }

This creates an internal private variable which you can't address directly, with the external property 'FooBar' to access it directly.

My question is - how often do you see this abused? It seems like it has a high potential to violate encapsulation best-practices often. Don't get me wrong, I use it as appropriate, and partial variations of it for read-only write-only types of properties, but what are your unpleasant experiences with it from other authors in your code base?

Clarification: the intended definition of abuse would indeed be creating such a property when private variables are appropriate.

BengtBe
  • 4,465
  • 1
  • 26
  • 19
dreadwail
  • 15,098
  • 21
  • 65
  • 96
  • Abused how exactly? the only abusive thing I can think of is putting a method's worth of work into a property, and that's fairly minor – annakata Jul 02 '09 at 09:31
  • 1
    Could you give an example where you think the use of automatic properties is not appropriate? – Dirk Vollmar Jul 02 '09 at 09:31
  • 1
    @annakata - which you **can't** do with an automatically implemented property. – Marc Gravell Jul 02 '09 at 09:32
  • Re clarification - if you create a **public** property when you meant a **private** field... then that is just a coding error. You might as well ask "how is while(true) {}" abused? You can have private auto-props, if you really want... – Marc Gravell Jul 02 '09 at 09:38

4 Answers4

34

I've seen it abused (in my opinion). In particular, when the developer would normally write:

private readonly int foo;
public int Foo
{ 
    get 
    { 
        return foo;
    }
}

they'll sometimes write:

public int Foo { get; private set; }

Yes, it's shorter. Yes, from outside the class it has the same appearance - but I don't view these as the same thing, as the latter form allows the property to be set elsewhere in the same class. It also means that there's no warning if the property isn't set in the constructor, and the field isn't readonly for the CLR. These are subtle differences, but just going for the second form because it's simpler and ignoring the differences feels like abuse to me, even if it's minor.

Fortunately, this is now available as of C# 6:

// Foo can only be set in the constructor, which corresponds to a direct field set
public int Foo { get; }
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Good point. Would also like to see readonly for automatic properties – BengtBe Jul 02 '09 at 09:35
  • I've seen this commonly as well, but your take is interesting. – dreadwail Jul 02 '09 at 09:37
  • Its not nearly the same, cause you marked the local variable foo as readonly. If it is readonly, it can't be set anywhere other as on first initialization, if not, it can be accessed from elsewhere in the same class - like the latter form of you. – Oliver Friedrich Jul 02 '09 at 09:38
  • I think beowulf's sideways making the point that it's the readonly part which makes this "abusive", which is pretty damned narrow – annakata Jul 02 '09 at 09:48
  • @annakata: I see it as an abuse because you're taking a shortcut to represent something which is *like* what you really mean, but isn't *actually* what you really mean. – Jon Skeet Jul 02 '09 at 10:32
  • I was also hoping for some way to mark the underlying field readonly, perhaps via an attribute, but it looks like this feature didn't make it to the C# 4 release. Sucks. – Steven Sudit May 26 '10 at 19:46
  • @Steven: I don't think it was ever part of any candidate set of features. At least, not one that I was privy to :) – Jon Skeet May 26 '10 at 19:52
  • I'm sure you're right, Jon, but it really ought to have been, because I'm guilty of routinely making exactly this mistake and wish there was an alternative that didn't require an explicit backing field. My conscience troubles me. – Steven Sudit May 26 '10 at 20:47
  • 2
    @Steven: Oh I totally agree. It's my #1 "this is really simple, please make it so" wish for C# 5. The team knows, and I'm hopeful - modulo the normal "even a small feature takes a lot of design, implementation and testing effort" caveat. – Jon Skeet May 26 '10 at 21:02
  • on the other hand, it is often used to make e.g. entities play well with o/r mappers, automapper etc and still behave as if there is no setters from an outside perspective.. – Roger Johansson Nov 28 '13 at 13:38
  • @RogerAlsing: When it's used *deliberately* for a specific reason, that's fine. I just wish it were a lot easier to handle the case of "I really want it to be totally readonly." – Jon Skeet Nov 28 '13 at 13:40
  • 2
    As an update on @JonSkeet's wish: read-only automatically implemented properties are now available as of C# 6.0. Simply exclude the `set;` portion of the standard syntax. You can also initialize them on the same line, by appending `= someValueExpression`. – Matthew Wirth Oct 17 '15 at 11:15
  • 2
    Example of the C# 6.0 syntax that @dreikin refers to, adapted from [link]http://stackoverflow.com/a/34743656/1991614 `public class Dog { public string Name { get; } public DateTime PuppyBorn { get; } = DateTime.Now; public Dog(string name) { Name = name; } }` – bitcoder Sep 05 '16 at 23:10
  • @JonSkeet In the C# 6 version, what if I would like to validate the value before assigning it? Should I do it in the constructor? My gut tells me not to since the constructor can get enormous, but I am not sure. – Michael Haddad Nov 21 '17 at 09:41
  • @Sipo: If it's passed into the constructor, validate it in the constructor. That's the case for all versions of C#, IMO. – Jon Skeet Nov 21 '17 at 10:05
  • @JonSkeet - Thanks. If I *do* need to use a private setter, will it be better to validate the value inside it rather than in the constructor? – Michael Haddad Nov 21 '17 at 10:07
  • 1
    @Sipo: Why do you believe you need a private setter? If it's so you can set it from places other than the constructor, then presumably you want validation in all of those places, so it makes sense to put it in the setter. If you're only using a private setter so you can cheat to make something look "somewhat readonly" then I'd do it properly and have a readonly field set by the constructor. (And work on upgrading to C# 6 or later...) – Jon Skeet Nov 21 '17 at 10:10
  • @JonSkeet - It seems to me like that is not the constructor responsibility, but apparently there is no other option. I did suggest to the C# team to add such a feature: https://visualstudio.uservoice.com/forums/121579-visual-studio-ide/suggestions/32268457-a-manually-implemented-setter-available-only-to-t – Michael Haddad Nov 21 '17 at 10:14
  • @Sipo: It's a parameter to the constructor, so it's reasonable for the constructor to validate it. But at this point we're a fair way from the subject of the question, and Stack Overflow is not intended for discussion. – Jon Skeet Nov 21 '17 at 10:21
3

There is no "abuse" in simply not writing the field manually; and it is good to encourage all access via the property (not directly to the field) anyway!

The biggest problem I know of is with binary serialization, where it gets a bit tricky to change back to a regular field without making it version-incompatible - but then... use a different serializer ;-p

It would be nice if there was a "proper" readonly variant, and if you didn't need to use :this() ctor-chaining on structs, but.... meh!

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • I agree. I think Properties and can help extend things later on (especially if they are public and you expect someone else to use your classes). – Ian Jul 02 '09 at 10:53
  • Follow the bouncing instruction pointer here ( ;) ): Automatic properties, you can't get to their backing stores. If it's eadonly then there's no way to give it a value *at all*. So thus it would be pointless... – RCIX Nov 13 '09 at 11:08
  • @RCIX: The idea would be to declare it as a private set, but use a `[Readonly]` to force this access to be constrained exactly as `readonly` is. – Steven Sudit May 26 '10 at 20:46
1

I haven't seen any abuse of it. And to be honest, I don't really see what you mean, as I can't see how this syntax could be abused.

Razzie
  • 30,834
  • 11
  • 63
  • 78
0

I don't think automatic properties are any worse than regular properties in regards to encapsulation.

If you mean that some developers use public automatic properties instead private fields then this is of course wrong and breaks encapsulation..

BengtBe
  • 4,465
  • 1
  • 26
  • 19