tags:

views:

168

answers:

2

Hi, I have a bit of a design dilemma at present. I have an abstract class Firmware which handles file transfer (updating the firmware) and a few other things such as version.

The trouble is that I want to update the file paths of all MyFirmwares in all Devices that I have over the place. One way I can do this is to have a static list of Devices in Device which I iterate over updating Device.FilePath, whenever I set FirmwareFilePath or get an event from Firmware that it's FilePath has changed, remember to clean them up an so on.

** Edit - made this example more complete*

public class Firmware
{
    private string _path;
    public string Path
    {
        get { return _path; }
        set
        {
            if (_path == value)
                return;

            _path = value;

            OnPropertyChanged("Path");
        }
    }
}

public class Device
{
    private static readonly List<Device> _Bars = new List<Device>();

    private readonly Firmware _myFirmware = new Firmware() ;
    public Firmware MyFirmware
    {
        get { return _myFirmware; }
    }

    public Device()
    {
        _Bars.Add(this);
        Firmware.PropertyChanged += NewPath;

    }

    private void NewPath (object sender, PropertyChangedEventArgs e)
    {
        if (e.PropertyName == "Path")
        {
            foreach (var dev in _Bars)
                dev.MyFirmware.Path = MyFirmware.Path;
        }
    }

}


Or I could use a "pointer" , change my Getter, Setter and Ctor slightly.

public class Pointer<TField>
{
  private TField _backingField;

  public TField GetValue()
  {
   return _backingField;
  }

  public void SetValue(TField value)
  {
   _backingField = value;
  }
 }

public class Firmware
{
  private Pointer<string> _pPath;
  public string Path
  {
   get { return _pPath.GetValue(); }
   set { _pPath.SetValue(value); }
  }

  public Firmware (Pointer<String> pPath)
  {
   _pPath = pPath;
  }
 }

 public class Device
 {
  private static readonly Pointer<String> _PPath = new Pointer<string>();
  public static string Path
  {
   get { return _PPath.GetValue(); }
   set { _PPath.SetValue(value); }
  }

  private readonly Firmware _myFirmware = new Firmware(_PPath);
  public Firmware MyFirmware
  {
   get { return _myDevice; }
  }
 }

Is there any agreed upon reason why this would be bad practice? Is there any GC trap I haven't noticed?

A: 

No, I can't see no GC traps and your solution seems allright although a little odd. What do you need the pointer for? What you're really looking for is a reference, as long as you change the properties of the referenced object all references will be updated too.

If you want to change the object instance itself (which might be the case), I thinks your solution seems about right.

Jorge Córdoba
Yes I'm trying to change all the instances of Firmware which belong to any Device object.
Courtney de Lautour
I can't see the necessity for changing the instance instead of just updating some of the properties of the object itself, that would update ALL the reference to that given object. Anyway your solution would work and, depending on the rest of your code, it could be easier to understand.
Jorge Córdoba
+1  A: 

Why? As best as I can follow, you want to store an instance of your pointer class on a bunch of objects scattered around all over the place so that they all have a "pointer" to your one main object, and if you update your main object, all the parent classes will be updated, too.

I have several questions:

  1. if Bar has a Foo member, and Foo has a FilePath, why does Bar have a FooFilePath? Any time you need FooFilePath, why not use MyFoo.FilePath? And why is FooFilePath static?
  2. In your second example, instead of each Bar having a MyFoo, it has a pointer - that's the only real thing you've changed. The pointer just has a member that serves the purpose that Foo.FilePath used to serve. So why?
  3. If you just shared an instance of Foo with all instances of Bar, and you changed the value of Foo.FilePath, and made sure that instead of using Bar.FooFilePath all users used Bar.MyFoo.FilePath, then if you change the value of FilePath on that shared Foo instance, every instance of Bar will get the new value automatically anyway, without you having to jump through these hoops.

The problem isn't that your solution won't work, but rather that it's unnecessary.

EDIT: Re: your comment. In that use case, yes, your pointer should work fine, though rather than using a generic pointer class, I'd probably create a class whose entire purpose is to hold shared data - just in case you need more than the file path later on, you already have a container for it. And instead of the double-indirection approach that you're using, you would just use a common reference to this shared instance in all classes that need it.

Dathan
1) Since that's where the actual value is stored when - it is static because all MyFirmware on a particular type of Device should have the same FilePath. 2) I'm not sure I understand, each device still has a Firmware, and as you say there is now a pointer. 3) Firmware objects contain more data than just FilePath - this stops me from sharing the same Firmware object.
Courtney de Lautour
Ah, that informs the discussion a bit. These are the pitfalls of creating toy examples that incompletely portray the actual architecture. See my edit.
Dathan