1
\$\begingroup\$

It is not related to GUI. I have a class with:

  1. Private List, which is edited inside the class.
  2. Open IReadOnlyCollection property for access to private List
  3. Public event for notification on List changing

A list stores objects with properties. The class can changed an item's properties and notify about it. The user can only read an item's properties. I only picked this option.

class MortarInfo
{
    public double Angle { get; private set; }
    public bool State { get; private set; }
    public MortarInfo(double angle, bool state)
    {
        Angle = angle;
        State = state;
    }
}
class Mortars
{
    private List<MortarInfo> _mortarsInfo;
    private ReadOnlyCollection<MortarInfo> _readOnlyMortarsInfo;

    public Mortars()
    {
        _mortarsInfo = new List<MortarInfo>();
        _readOnlyMortarsInfo = _mortarsInfo.AsReadOnly();
    }

    public ReadOnlyCollection<MortarInfo> MortarsInfo
    {
        get { return _readOnlyMortarsInfo; }
    }
    public event ListChangedEventHandler MortarsInfoChanged;

    public void UpdateAngle(int index, double newAngle)
    {
        if (_mortarsInfo[index].Angle != newAngle)
        {
            _mortarsInfo[index] = new MortarInfo(newAngle, _mortarsInfo[index].State);
            if (MortarsInfoChanged != null)
            {
                var arg = new ListChangedEventArgs(ListChangedType.ItemChanged, index);
                MortarsInfoChanged(this, arg);
            }
        }
    }
}
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

It seems good but I have some comments on this,

  1. Why do you create a new ListItem in every property change and not just replace to old property with the new one?

    _list[index].Property1 = property1;
    
  2. You raise the event without null check. Do this:

    var listChanged = ListChanged;
    if (listChanged != null)
        listChanged(this,new ListChangedEventArgs(ListChangedType.ItemChanged, index));
    
  3. As a side note, if Property1 or Property2 are complex objects, It's recommended to create for them you own equal methods (Equal, GetHashCode, == and !=). If the are generic, implement IEquatable<T>

\$\endgroup\$
2
  • \$\begingroup\$ 1. I create a new ListItem, because user don't to be able to hange anything. 2. In project i use Extension methods to raise event. It have a check. 3. It a good idea. \$\endgroup\$
    – Nodon
    Dec 23, 2015 at 8:43
  • 1
    \$\begingroup\$ It'd be helpful to explain why the temporary variable is needed in the null check. I.e. The handler could go null between the check and call without it. \$\endgroup\$
    – RubberDuck
    Dec 23, 2015 at 9:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.