Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. Dismiss Notice

Need to extended C# List<T> and keep default List<T> inspector

Discussion in 'Scripting' started by AdamRamberg, May 26, 2018.

  1. AdamRamberg

    AdamRamberg

    Joined:
    Dec 8, 2016
    Posts:
    22
    Howdy!

    I want to extend a C# List like this:
    Code (CSharp):
    1. public class ObservableList<T> : List<T>
    2. {
    3.     public event Action<T> OnAdd;
    4.     public event Action<T> OnRemove;
    5.  
    6.     public new void Add(T item)
    7.     {
    8.         if (null != OnAdd)
    9.         {
    10.             OnAdd(item);
    11.         }
    12.         base.Add(item);
    13.     }
    14.  
    15.     public new void Remove(T item)
    16.     {
    17.         if (null != OnRemove)
    18.         {
    19.             OnRemove(item);
    20.         }
    21.         base.Remove(item);
    22.     }
    23.  
    24. }
    How do I use this class in a Monobehaviour and get the default C# List<T> inspector? Was first thinking of creating my own PropertyDrawer, but that will only work for one Type eg. a property drawer for ObservableList<int>. I would like a solution that works for all types and that doesn't require me to create a property drawer for each one. Is there for example a way to just display the default list's inspector?

    Thanks,
    Adam
     
  2. Suddoha

    Suddoha

    Joined:
    Nov 9, 2013
    Posts:
    2,824
    I would use a wrapper instead. It does almost look the same and does not require additional code.

    Here's the reason why:

    Add, remove and other methods cannot be overridden, I guess that's why you've decided to hide them with new implementations instead.

    Hiding members with the new-keyword introduces behaviour that breaks polymorphism, i.e. if you assigned an instance of your new list type to a variable of a more generalized type, such as List<T> or IList<T>, it'd behave differently even though you'll probably expect polymorphic behaviour.

    I doubt that's what you want.

    Some additional notes:

    1) I'd expect other methods to raise the events as well. For example, RemoveAt, Insert, the indexer [..] and all that stuff. Perhaps a seperate event for the Clear() method, as you probably don't want to raise it for every single instance. yet 'clearing' is some sort of removing, too.

    That's only my opinion though, as consistency may matter sooner or later.

    2) The events are a little unclear, but that's something you have to document.

    Anyway, here are my thoughts: For someone who simply takes a look at the classes interface, does "OnAdd" mean it has already been added when the event is raised, i.e. if I accessed the list in a listener, would Contains() return true or false?
    Same applies to OnRemove.

    At the moment, OnAdd and OnRemove are raised before the actual action takes place (internally). If I accessed the source / sender (which could be added here as well as a parameter) I would be confronted with state that doesn't seem to have proper integrity, depending on how my subjective interpretation is - it requires documentation and might not even be consistent throughout your entire code-base.

    For this reason, I prefer (and recommend) Microsoft's convention / recommendation, which includes clear tenses. The events could then be named like ItemAdded (which suggests it is added before the event is raised) and ItemRemoved (which suggest is it removed before the event is raised).

    It's some sort of nitpicking, but this adds a lot to understanding code and behaviour a lot better.

    In the end, that's up to you though.
     
    Peter77 likes this.
  3. AdamRamberg

    AdamRamberg

    Joined:
    Dec 8, 2016
    Posts:
    22
    Thanks for the reply! You are correct that there are some unwanted behaviour using the new keyword (hiding Add / Remove). I guess wrapping the list might be an ok solution (even though it will be grouped under the new wrapper in the inspector when used in a MonoBehaviour, which is not the cleanest look).

    You have some additional valid points, but it was not really in the scope of this question :)

    Would still like to know if there is a way in my example to get the default List<T> inspector for my extended class.