Field should be defined public

[Tony Johansson]

Hello!

This class Card displayed below is from a book called "Visual Studio 2005".
Now to my question here they haved used accessibility of public for the two
field suit and rank
This is completely wrong I changed it to private and the example application
containing this class Card works just as good as having the accessibility
set to public.
Do you agree with me?
There is almost never acceptable to have a field set to public.

namespace Ch10CardLib
{
public class Card
{
public readonly Suit suit;
public readonly Rank rank;

private Card() {}

public Card(Suit newSuit, Rank newRank)
{
suit = newSuit;
rank = newRank;
}

public Rank Rank
{
get
{
throw new System.NotImple mentedException ();
}
set
{
}
}

public Suit Suit
{
get
{
throw new System.NotImple mentedException ();
}
set
{
}
}

public override string ToString()
{
return "The " + rank + " of " + suit + "s";
}
}
}


//Tony

[Jon Skeet [C# MVP]]

Re: Field should be defined public

Tony Johansson <johansson.ande rsson@telia.com wrote:

This class Card displayed below is from a book called "Visual Studio 2005".
Now to my question here they haved used accessibility of public for the two
field suit and rank
This is completely wrong I changed it to private and the example application
containing this class Card works just as good as having the accessibility
set to public.
Do you agree with me?
There is almost never acceptable to have a field set to public.

Agreed, for production code. Occasionally it's simpler to use public
variables when trying to teach a completely different point. Perhaps
that's what was happening here?

For instance, judging by the code it *looks* like the author is
gradually introducing properties - migrating *from* public fields to
public properties and private fields. That's reasonable, but it would
be good for the author to explain up-front that public fields *are* bad
in general, even if they're used for some examples.

--
Jon Skeet - <skeet@pobox.co m>
Web site: http://www.pobox.com/~skeet
Blog: http://www.msmvps.com/jon.skeet
C# in Depth: http://csharpindepth.com

[=?ISO-8859-1?Q?Arne_Vajh=F8j?=]

Re: Field should be defined public

Tony Johansson wrote:

This class Card displayed below is from a book called "Visual Studio 2005".
Now to my question here they haved used accessibility of public for the two
field suit and rank
This is completely wrong I changed it to private and the example application
containing this class Card works just as good as having the accessibility
set to public.
Do you agree with me?
There is almost never acceptable to have a field set to public.

I agree.

But books are far from perfect.

Arne

[Peter Duniho]

Re: Field should be defined public

On Sat, 07 Jun 2008 04:18:05 -0700, Tony Johansson
<johansson.ande rsson@telia.com wrote:

Hello!
>
This class Card displayed below is from a book called "Visual Studio
2005".
Now to my question here they haved used accessibility of public for the
two
field suit and rank
This is completely wrong I changed it to private and the example
application
containing this class Card works just as good as having the accessibility
set to public.
Do you agree with me?
There is almost never acceptable to have a field set to public.

In addition to the other replies, both of which I agree with, I'll note
that the public fields are "readonly". Properties still have advantages
over this approach, but making them "readonly" ensures that the fields
meet at least one of the benefits of properties: to encapsulate the value
in a way that prevents code from outside the class from messing with it
(and in fact, does the same for code within the class :) ).

In the code you posted, there are _also_ properties associated with those
fields, so I'd think that Jon's speculation that the book may be in the
process of introducing properties is likely to be a valid assessment. But
a "readonly" field can be a handy shortcut for specific scenarios
(especially demonstration code).

In other words, from an encapsulation point of view, if you are sure
you're never going to need an actual property (for binding, for example),
a "readonly" field offers basically the same functionality. Prior to C#
3, this syntax was a convenient way to declare a public member to contain
a value without all the extra overhead of declaring a property.

Of course, now you can just write:

public Suit suit { get; }

and it works pretty much the same, plus with the stuff properties give
you. But maybe the book was written before C# 3.

Pete

[Jon Skeet [C# MVP]]

Re: Field should be defined public

On Jun 7, 8:30 pm, "Peter Duniho" <NpOeStPe...@nn owslpianmk.com>
wrote:

<snip>

Of course, now you can just write:
>
public Suit suit { get; }
>
and it works pretty much the same, plus with the stuff properties give
you. But maybe the book was written before C# 3.

Unfortunately, you *can't* write read-only automatic properties in C#
3. You can write properties like this:

public Suit Suit { get; private set; }

But you can't write automatically implemented properties which have
the same semantics as readonly variables - i.e. only settable within
the constructor. I'm hoping we might get them for C# 4 :)

Jon

[Peter Duniho]

Re: Field should be defined public

On Sat, 07 Jun 2008 16:16:04 -0700, Jon Skeet [C# MVP] <skeet@pobox.co m>
wrote:

Unfortunately, you *can't* write read-only automatic properties in C#
3.

Ahh...I probably ran into that already and just forgot. Oh well. Even
more justification for the original syntax shown, however marginal. :)