0

Let's say I've got a Car class and that class contains a Radio object. Which looks like

class Radio
    {
        public string Model { get; set; }
        private List<string> myChannels = new List<string>();

        public List<string> PresetChannels
        {
            get { return myChannels; }
            set { myChannels = value; }
        }

        private bool radioState;

        public void ToggleRadio()
        {
            if (!radioState)
                radioState = true;
            else
                radioState = false;
        }
        private string selectedChannel = string.Empty;
        //
        public void SetStation(int radioButton, string channelName)
        {
            while (!ValidateRadioButtonNumber(radioButton))
            {
                Console.Write("Index out of range, choose another value: ");
                radioButton = Convert.ToInt32(Console.ReadLine());
            }

            PresetChannels[radioButton] = channelName;
            Console.WriteLine("The {0} radio button was set to {1}",radioButton,channelName);
        }
        private bool ValidateRadioButtonNumber(int radioButton)
        {
            if (radioButton < 0 || radioButton > 5)
                return false;
            else
                return true;
        }
        //
        public void SelectChannel(int radioButton)
        {
            while (!ValidateRadioButtonNumber(radioButton))
            {
                Console.Write("Index out of range, choose another value: ");
                radioButton = Convert.ToInt32(Console.ReadLine());
            }
            selectedChannel = PresetChannels[radioButton];
            Console.WriteLine(PresetChannels[radioButton]);
        }
        public Radio()
        {
            PresetChannels = new List<string>();
            PresetChannels.Capacity = 5;
            //initialize every element in the list at runtime
            //so the user can set any station they wish
            for (int i = 0; i < PresetChannels.Capacity; i++)
            {
                PresetChannels.Add(string.Empty);
            }
        }

    }

with the Car class like

public class Car
{
    public int Year { get; set; }
    public string Model { get; set; }
    private Radio radio;

    public Radio MyRadio { get; set; }

    //initialize the radio of the car
    public Car()
    {
        radio = new Radio();
        MyRadio = new Radio();
    }
    //containment
    public void SelectStation(int radioButton)
    {
        radio.SelectChannel(radioButton);
    }
    public void SetStation(int radioButton, string channelName)
    {
        radio.SetStation(radioButton, channelName);
    }
    public void ToggleRadio()
    {
        radio.ToggleRadio();
    }
}

If I make design the class with MyRadio as a property, then what's the point of containment? If a property of Radio had a private setter and you tried to set that value in the Main method it wouldn't compile, right?

            Car c = new Car();
            c.SetStation(0, "99.2");
            c.SetStation(10, "100"); //works
            c.SelectStation(120);
            Car c2 = new Car();
            c2.MyRadio.SetStation(0, "99.0");//works
            Console.ReadLine();

What are some general guidelines as to when one should keep a custom type a field vs. making it a property?

Paul Sullivan
  • 2,865
  • 2
  • 19
  • 25
wootscootinboogie
  • 8,461
  • 33
  • 112
  • 197
  • 3
    Make it a property if you want it modified externally, or a field if it should only be modified internally. – Mansfield Jul 16 '13 at 17:53
  • 1
    Properties are a subset of members. As are fields. – Kirk Woll Jul 16 '13 at 17:55
  • possible duplicate of [Auto-implemented getters and setters vs. public fields](http://stackoverflow.com/questions/111461/auto-implemented-getters-and-setters-vs-public-fields) – nawfal Jul 17 '14 at 20:40

2 Answers2

3

It seems like overkill in this case, to have a Radio in your car and then provide methods around it for accessing it. Personally, I'd just have a Radio property with a getter that returned the radio instance, and the caller could then work with the radio directly.

Should you need communication with the Car during this process, you can do something like:

public class Radio
{
    public delegate void StationEvent(int channel);

    private int _station;

    public int Station
    {
        get { return _station; }
        set 
        {
            _station= value;
            if(SetStation != null)
                SetStation(_station);
        }
    }


    public event StationEvent SetStation;
    // etc...
}


public class Car
{
    private Radio _radio = new Radio();

    public Car()
    {
        _radio.SetStation += (station) => { /* Set station was called */ };
    }

    public Radio Radio { get { return _radio; } }
}

Now the caller of the Car instance can work with the radio, and the car can be notified of events. Hell, anything can be notified of a Radio's events.

Moo-Juice
  • 38,257
  • 10
  • 78
  • 128
  • I was disappointed that I received no compliments on my username..until now. I thought it was a stroke of hilarity :). Back to the matter at hand, could you explain what you mean by "It seems like overkill in this case, to have a Radio in your car and then provide methods around it for accessing it." – wootscootinboogie Jul 16 '13 at 18:03
  • Well, there's no point having a property in your car to expose the radio and then some (rather wordy) methods to work with it, when it's far simpler (imho) to let people work with the Radio directly and avoid all those method calls. From a logical point of view, when you go and change the station on your radio, do you work with the radio directly, or the car? (I am aware some more complex radios are quite integral to the car :) ) – Moo-Juice Jul 16 '13 at 18:05
  • that makes sense enough to me. Strictly from a readability standpoint c.MyRadio.SetStation reads better to me than c.SetStation – wootscootinboogie Jul 16 '13 at 18:08
2

The answer to your question is probably a bit subjective.

What are some general guidelines as to when one should keep a custom type a field vs. making it a property?

However, Microsoft does suggest some guidelines here. In a nutshell use a property if you'll want to do validation or otherwise control how/what is set when you set the value, or if there are side effects.

You might also want to check here, here, or here.

Community
  • 1
  • 1
Steve
  • 7,747
  • 5
  • 31
  • 44