Field should be defined public

T

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.NotImplementedException();
}
set
{
}
}

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

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


//Tony
 
J

Jon Skeet [C# MVP]

Tony Johansson said:
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.
 
A

Arne Vajhøj

Tony said:
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
 
J

Jon Skeet [C# MVP]

On Jun 7, 8:30 pm, "Peter Duniho" <[email protected]>
wrote:

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
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top