views:

118

answers:

6

In my app, I'm working with Simfile and Notechart objects. A Simfile is essentially a Notechart container, with the following constraints:

1) Every Notechart must be contained in exactly one parent Simfile, at all times.
2) Given a Simfile object, I need to be able to obtain all of its contained Notecharts (should be easy).
3) Given a Notechart, I need to be able to obtain its parent Simfile (harder).

My current implementation is using a Dictionary as the Simfile base class, since each Notechart has a unique key, at least within a Simfile. The problem I'm facing is how to enforce that a Notechart always has a parent Simfile? Right now, I'm able to create a Notechart independently of a Simfile, and add it to the Simfile dictionary without making the needed reference from Notechart -> Simfile.

Ideally, the best resolution would be an Add() method in Simfile that only can be called from within Notechart's constructors, but I don't think that's possible.

I want to say that this is a common problem, but it's the first time in practice I've come across it, and I'm not sure what it's called.

+1  A: 

One way to achive this is not allow external callers to create instances of NoteChart, and have a method on the Simfile which creates and returns instances of NoteChart that are already wired to the Simfile instance.

Here is a simplistic example, note I did not add any error checking for duplicate IDs etc.

public class NoteChart
{
  public SimFile Owner {get; private set;}
  public string Id { get; private set; }

  internal NoteChart(SimFile owner, string id)
  {
    this.Owner = owner;
    this.Id = id;
    Owner.Add(id, this);
  }
}

public class SimFile : Dictionary<string, NoteChart>
{
  public NoteChart CreateNoteChart(string id)
  {
    NoteChart noteChart = new NoteChart(this, id);
    return noteChart;
  }
}

Using this would then be something like the following

  // Get instance of SimFile somehow. I just newed it here for the example
  SimFile sim = new SimFile();

  // Create a new NoteChart on the SimFile
  NoteChart chart = sim.CreateNoteChart("NoteChart-1");

The thing here is that the NoteChart constuctor is marked internal. That means that other classes in other assemblies will not be able to directly construct instances of the NoteChart class and will be forced to work through the SimFile.CreateNoteChart method.

Within the same assembly, the code can still call the internal constructor, but will have to pass the owning SiteFile and Id, and the constructor will add the NoteChart to the SimFile.

You might want to reconsider the design choice of inheriting SimFile from a Dictionary, it might be better to just have a Dictionary as a member and expose the funtionality you require. For example now a NoteChart could inadvertantly be added directly to another SimFile instance through the SimFle.Add which is inherited from Dictionary, and with a mismatched Id. But that is just some food for thought.

Chris Taylor
This would be good, but the Simfile's Add() method would still be publicly exposed, and there's nothing to stop another programmer from adding a Notechart created from one Simfile to a different Simfile instance. Ideally, the way to do it would be an Add() method in Simfile that could only be called when inside Notechart's constructor, but I don't think that's possible.
Mark LeMoine
@Mark, That is correct and also the reason for the suggestion in the last paragraph of my answer "You might want to reconsider the design choice of inheriting SimFile from a Dictionary...". Of course the dictionary should be a private.
Chris Taylor
+2  A: 

You can use code structure similar to this:

public class Simfile
{
   public Simfile()
   {
      Notecharts = new List<Notechart>();
   }
   public List<Notechart> Notecharts { get; private set; }
}

public class Notechart
{
   public Notechart(Simfile simfile)
   {
      Simfile = simfile;
      simfile.Notecharts.Add(this);
   }
   public Simfile Simfile { get; private set; }
}

Only thing you need to make sure of is that every other constructor you add to Notechart class has to take a Simfile argument.

EDIT (based on the comment)


If you don't want to ensure that Notechart can't be added to Simfile if it isn't it's parent simfile you can do this change:

public class Simfile
{
   public Simfile()
   {
      ChildNotecharts = new List<Notechart>();
   }
   private List<Notechart> ChildNotecharts { get; set; }
   public IEnumerable<Notechart> Notecharts
   {
      get { return ChildNotecharts; }
   }
   public int AddNotechart(Notechart notechart)
   {
      if (ChildNotecharts.Contains(notechart)) return 0;
      if (notechart.Simfile != this) return -1;
      ChildNotecharts.Add(notechart);
      return 1;
   }
}

public class Notechart
{
   public Notechart(Simfile simfile)
   {
      Simfile = simfile;
      simfile.AddNotechart(this);
   }
   public Simfile Simfile { get; private set; }
}
Ivan Ferić
The thing is though, I'd still be able to add a Notechart to a Simfile by calling Simfile.Notecharts.Add(). And if that Notechart is already tied to a different parent Simfile, that'd be bad.
Mark LeMoine
I edited the answer based on your input. Check it out now.
Ivan Ferić
+1  A: 

Here are two possible solutions out of quite a few.

Have a constructor in NoteChart that accepts a Simfile object, and calls a function within the Simfile object that adds the NoteChart. If you have such a parameterized constructor, the default constructor is no longer available. Something like this:

public class SimFile
{
    public void AddNoteChart(NoteChart nc)
    {
        // Here you can add the note chart to the SimFile dictionary
    }
}

public class NoteChart
{
    public SimFile PapaSimFile { get; private set; }

    NoteChart(SimFile simFile)
    {
        if (simFile == null)
        {
            // throw exception here
        }
        PapaSimFile = simFile;
        PapaSimFile.AddNoteChart(this);
    }
}    

Another option would be to make NoteChart accessible only from within SimFile. For example:

public class SimFile
{
    public class NoteChart
    {
        public SimFile SimFile { get; internal set; }
    }

    public NoteChart CreateNoteChart()
    {
        NoteChart nc = new NoteChart();
        nc.SimFile = this;
        // Here you can add the note chart to the SimFile dictionary
        return nc;
    }
}

There are advantages and disadvantages to each approach, you should look at the whole picture in order to decide which is better.

Eldad Mor
Nesting `NoteChart` inside `SimFile` does not make it less accessible to outside code when both are marked `public`. Your second solution is exactly as accessible as it would be if `NoteChart` were declared separately. (`internal` means that all the code within the same assembly can access it.)
Timwi
Thanks Timwi! Good to know.
Eldad Mor
+1  A: 

Your main requirement seems to be that you don’t want outside code to modify the SimFile except by creating new NoteCharts. So we definitely need to:

  • restrict access to the NoteChart constructor (otherwise you could have a NoteChart instance with a parent SimFile that doesn’t actually contain it)

  • restrict access to the SimFile dictionary (otherwise other code could add things to it)

  • allow access to the read-only abilities of the SimFile dictionary.

Here’s how I would do it.

public class NoteChart
{
    public SimFile Parent { get; private set; }
    public string Id { get; private set; }

    // internal: code outside this assembly cannot instantiate this class
    internal NoteChart(SimFile parent, string id)
    {
        this.Parent = parent;
        this.Id = id;
        Parent.dictionary.Add(id, this);
    }
}

// Implement only IEnumerable, as that is read-only;
// all the other collection interfaces are writable
public class SimFile : IEnumerable<KeyValuePair<string, NoteChart>>
{
    // internal: not accessible to code outside this assembly
    internal Dictionary<string, NoteChart> dictionary =
        new Dictionary<string, NoteChart>();

    // Public method to enable the creation of a new note chart
    // that is automatically associated with this SimFile
    public NoteChart CreateNoteChart(string id)
    {
        NoteChart noteChart = new NoteChart(this, id);
        return noteChart;
    }

    // Read-only methods to retrieve the data. No writable methods allowed.
    public NoteChart this[string index] { get { return dictionary[index]; } }
    public IEnumerator<KeyValuePair<string, NoteChart>> GetEnumerator() {
        return dictionary.GetEnumerator(); }
    public int Count { get { return dictionary.Count; } }
    public IEnumerable<string> Keys { get { return dictionary.Keys; } }
    public IEnumerable<NoteChart> Values { get { return dictionary.Values; } }
}
Timwi
Is there any way to make the has have "stricter than internal" access? I wouldn't want other classes in the same assembly other than Simfile be able to create Notecharts.
Mark LeMoine
@Mark: I’ve run into this before and I too find it annoying that there is nothing stricter. You can make one of them (the constructor *or* the dictionary) private and then nest the other class inside it, but you can’t restrict both.
Timwi
A: 

The following implements a Notechart that always has a parent SimFile. It works by hiding the container (to keep things from being added to a SimFile without the parent changing) and by making the Notechart a part of the SimFile class so that it can access the private collection when its parent is changed. This is natural since your requirement is that Notecharts not exist without a corresponding SimFile; they are truly a part of SimFiles.

public class SimFile
{
    private List<Notechart> _notecharts = new List<Notechart>();

    public IEnumerable<Notechart> Notecharts 
    {
        get {return _notecharts.AsEnumerable<Notechart>();}
    }

    public class Notechart
    {
        public Notechart(SimFile parent)
        {
            this.Parent = parent;
        }

        private SimFile _parent = null;
        public SimFile Parent
        {
            get {return _parent;}
            set 
            {
                if (value != null)
                {
                    if (_parent != null)
                    {
                        _parent._notecharts.Remove(this);
                    }
                    _parent = value;
                    _parent._notecharts.Add(this);
                }
                else
                {
                    throw new Exception("A Notechart MUST have a parent SimFile");
                }
            }
        }
    }
}
Cirdec
A: 

Well, I figured it out. I made the Notechart class abstract (fully implemented, but abstract to prevent other classes from instantiating it), and then made an internal class in Simfile derived from it (NotechartWorking) with protected constructors. I then followed the suggestions on here to have Simfile generate Notecharts for me. So, I was creating NotechartWorking objects, but returning them as Notecharts, which ended up working well.

Mark LeMoine