Search Unity

Why can't I pass a List of a derived type from an abstract class into method paramter

Discussion in 'Scripting' started by SprayNpraY, Jul 1, 2022.

  1. SprayNpraY

    SprayNpraY

    Joined:
    Oct 2, 2014
    Posts:
    156
    Hi I'm making a positive habit app and I'm trying to reduce repetitive code by maing an abstract class for positive and negative habits.

    I then made a method that takes a list and dictionary of type of the abstract class to allow me to use the derived types to further reduce repetitive code.

    But for some reason it isn't working.

    Here is the code:
    Code (CSharp):
    1.   abstract class Activity
    2.     {
    3.         [SerializeField] string _activity;
    4.         [Range(0, 10)]
    5.         [SerializeField] int _value = 1;
    6.  
    7.         public string ActivityName => _activity;
    8.         public int Value => _value;
    9.  
    10.         protected Activity(string activity, int value)
    11.         {
    12.             _activity = activity;
    13.             _value = value;
    14.         }
    15.  
    16.     }
    17.  
    18.     [System.Serializable]
    19.     class RewardingActivity: Activity
    20.     {
    21.         public RewardingActivity(string activity, int value) : base(activity,  value)
    22.         {          
    23.         }
    24.     }
    25.  
    26.     [System.Serializable]
    27.     class PunishmentingActivity: Activity
    28.     {
    29.         public PunishmentingActivity(string activity, int value) : base(activity, value)
    30.         {
    31.         }
    32.     }
    33.  
    34.    void FillDropDownsAndActivities(List<Activity> activities,
    35.         List<string> activityNames, Dictionary<int, Activity> activityDic, TMP_Dropdown[] dropDowns )
    36.     {
    37.         for (int i = 0; i < activities.Count; i++)
    38.         {
    39.             activityNames.Add($"{activities[i].ActivityName}" +
    40.                 $" RP:{activities[i].Value}");
    41.             activityDic.Add(i, activities[i]);
    42.         }
    43.  
    44.         foreach (var dropDown in dropDowns)
    45.         {
    46.             dropDown.options.Clear();
    47.             dropDown.AddOptions(activityNames);
    48.         }
    49.     }
    Now the problem is with trying to call FillDropDownsAndActivities, when I try to pass either a list or dictionary of one of the derived types I'm getting errors like:

    Error CS1503 Argument 1: cannot convert from 'System.Collections.Generic.List<ActivityScore.RewardingActivity>' to 'System.Collections.Generic.List<ActivityScore.Activity>' Assembly-CSharp, Assembly-CSharp.Player

    Can someone please explain why this is happening, and how to/suggestions for resolving it.
     
  2. RadRedPanda

    RadRedPanda

    Joined:
    May 9, 2018
    Posts:
    1,647
    Like the error says, C# can't convert one typed list to another, even though it's a child type.

    One thing you can do is maybe use a Generic List for a parameter, it would look something like this

    Code (CSharp):
    1.    void FillDropDownsAndActivities<T>(List<T> activities, List<string> activityNames, Dictionary<int, T> activityDic, TMP_Dropdown[] dropDowns ) where T : Activity
    2.     {
    3.         for (int i = 0; i < activities.Count; i++)
    4.         {
    5.             activityNames.Add($"{activities[i].ActivityName}" +
    6.                 $" RP:{activities[i].Value}");
    7.             activityDic.Add(i, activities[i]);
    8.         }
    9.         foreach (var dropDown in dropDowns)
    10.         {
    11.             dropDown.options.Clear();
    12.             dropDown.AddOptions(activityNames);
    13.         }
    14.     }
    Not 100% that this code would work, you might have to look into how Generics function.
     
    SprayNpraY and lordofduct like this.
  3. SprayNpraY

    SprayNpraY

    Joined:
    Oct 2, 2014
    Posts:
    156
    I get that, but why can't it convert when its a child of the abstract class that I'm passing?

    When child classes work in place of a parent class in most other contexts that I'm aware of.
     
  4. RadRedPanda

    RadRedPanda

    Joined:
    May 9, 2018
    Posts:
    1,647
    From what I gathered on the internet, List does not support something called Covariance, which I can probably not explain better than anyone else. You can however, use IEnumerable to do what you wanted, as IEnumerable does support Covariance.
     
    SprayNpraY likes this.
  5. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,532
    This is called covariance.
    https://docs.microsoft.com/en-us/dotnet/standard/generics/covariance-and-contravariance

    Specifically covariance of generics in your specific case.

    The problem that arises isn't just that the types derive, it's that the type wrapping the generic is read/write, causing covariance to fail.

    See... your method accepts List<Activity>. A List<Activity> has an 'Add' method. This means that your FillDropDownsAndActivities could technically add an Activity to the list... but since the list can't actually have Activity in it that would be a problem. The compiler can not deal with that and thusly it blocks you from doing it.

    IEnumerable<Activity> on the other hand is not read/write, it's read only. So thusly the covariance requirements are met and it will successfully work.

    Now you may notice array will support covariance like this just like IEnumerable. This is because array is specially supported by the compiler. And at runtime if you try to insert the wrong typed object into it, it will throw the ArrayTypeMismatchException. This behaviour exists legacy wise as it was basically the primary version of covariance in early C# before generics was introduced.

    It could be argued that the compiler could let you pass it in covariantly if and only if the called method doesn't attempt to write to the collection and instead only reads from it. And in theory this is possible at the cost of being more complicated on the compilers part. But... that's not how .net do. A choice was made, and this was that choice.

    Note, there is another type that lets you support Count and [index] accessor:
    https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.ireadonlylist-1?view=net-6.0
    IReadOnlyList<T>

    Have your method accept that, and you're explicitly telling the compiler "I want a list, but I plan to treat it exclusively read-only" thusly satisfying the covariance policy.
     
    Last edited: Jul 1, 2022
    SprayNpraY likes this.
  6. SprayNpraY

    SprayNpraY

    Joined:
    Oct 2, 2014
    Posts:
    156
    Thank you I thought that might be the case, I've had a similar issue before and looked into covariance and contravairance in the past but I'm still struggling to fully understand it, including your response.

    I read the links you've posted but unfortunately, I'm still not quite getting it, I changed the parameter for the method instead of list to an inenumerable and will change the for to a foreach loop.

    However, I can't figure out what I can do about the dictionary that also is required by the FillDropDownsAndActivities method.
     
  7. RadRedPanda

    RadRedPanda

    Joined:
    May 9, 2018
    Posts:
    1,647
    Using the Generics method I mentioned above would have solved that issue.
     
  8. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,532
    Well, technically if your dictionary was treated readonly there is the IReadOnlyDictionary that Dictionary implements allowing for the readonly covariance.

    BUT, your code adds to the dictionary. This means you can't use that.

    As @RadRedPanda points out, a generic method will facilitate that.

    This does open another rabbit hole in covariance which may or may not impact you.

    If we used @RadRedPanda 's example of:
    Code (csharp):
    1. void FillDropDownsAndActivities<T>(List<T> activities,
    2.     List<string> activityNames,
    3.     Dictionary<int, T> activityDic,
    4.     TMP_Dropdown[] dropDowns ) where T : Activity
    5. {
    6. ...
    Well if you tried calling it like so:
    Code (csharp):
    1. List<Activity> lst = /*some list of Activity*/;
    2. Dictionary<int, RewardingActivity> dict = /*some dict of RewardingActivity, which inherits from Activity */;
    3. FillDropDownsAndActivities(lst, someStringList, dict, someDropDownTmps);
    You'll get an error saying it can't infer what <T> is... so you'll end up having to do either:
    Code (csharp):
    1. FillDropDownsAndActivities<Activity>(lst, someStringList, dict, someDropDownTmps);
    Which will error because dict doesn't meat the covariance requirements.

    Or
    Code (csharp):
    1. FillDropDownsAndActivities<RewardingActivity>(lst, someStringList, dict, someDropDownTmps);
    and lst will error because it doesn't meet the contravariance requirements.

    But as long as the list and the dictionary have the same types, it doesn't matter. But, if you wanted to avoid it... this is how you really should set it up which makes your method covariant capable while accepting the write functionality of the dictionary.

    Code (csharp):
    1. void FillDropDownsAndActivities<T>(IReadOnlyList<T> activities, List<string> activityNames, Dictionary<int, T> activityDic, TMP_Dropdown[] dropDowns ) where T : Activity
    With this you're golden. As long as the list is a type T that inherits from the type used as the TValue in Dictionary, you're good to go.

    ...

    Though I will say... this is a very convoluted scenario of covariance brought on. Why are you even calling this funciton with Dictionary's of inherited types of Activity? There sounds to me like a hyper reliance on Dictionary to perform generalized functionality that could be better implemented through concrete types.

    But I mean... that's just a completely different discussion.

    My point is... in all my years of coding, which are a lot, I've never coded myself into this bizarro scenario of covariance/contravariance. Most people haven't. And I'm not surprised that you're having a hard time wrapping your head around the problems of it because the topic as a whole isn't exactly intuitive.
     
  9. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,532
    To kind of get to a point about what I mean about this being convoluted... and mind you I'm really sick right now (got the covid this week... ugh... so mind me if I'm rambly and incoherent).

    Your function is weird...

    It takes 4 parameters:
    A list of activities
    A list of strings
    A dictionary of int to activities
    An array of dropdowns

    And they're used for different things.

    First these 2 parameters make complete sense to me.
    activites - used to read from, this appears to be the primary subject of the function... the activities being put into the dropdown
    dropDowns - the target that the activities need to go into

    What are these for though?
    activityNames - gets filled with the generated strings that will be used in the dropdown... but why is it passed in as a parameter? What happens if I pass in a non-empty list? Entries will be added to the dropdown that this function doesn't account for. Is this an expected feature?

    activityDic - this ones even weirder... what is the point of this? All you've done is map the index to the activity as laid out in the 'activities' list. Why is that needed? The calling method has that list!!! And a List already maps index to activity! And then what happens if you pass in a non-empty dictionary? Add will throw an exception if you attempt to add an existing key.

    Basically... from a code review perspective. This function is completely problematic with so many opportunities to fail in an unintuitive manner. At the very least, if this implementation was unavoidable there should be checks on it that make sure these 2 collections are empty. And some documentation as to what the purpose is.

    Regardless though... why can't this method merely be:

    Code (csharp):
    1. void FillDropDownsAndActivities(IEnumerable<Activity> activities, IEnumerable<TMP_Dropdown> dropdowns)
    2. {
    3.     var lst = new List<string>();
    4.     foreach(var a in activities)
    5.     {
    6.         lst.Add($"{a.ActivityName}" +
    7.             $" RP:{a.Value}");
    8.     }
    9.     foreach(var dropdown in dropdowns)
    10.     {
    11.         dropdown.options.Clear();
    12.         dropdown.AddOptions(lst);
    13.     }
    14. }
    ??

    You know... KISS. Keep It Simple Stupid.

    If you really need the calling code to have that list of string. Return the list:
    Code (csharp):
    1. List<string> FillDropDownsAndActivities(IEnumerable<Activity> activities, IEnumerable<TMP_Dropdown> dropdowns)
    2. {
    3.     var lst = new List<string>();
    4.     foreach(var a in activities)
    5.     {
    6.         lst.Add($"{a.ActivityName}" +
    7.             $" RP:{a.Value}");
    8.     }
    9.     foreach(var dropdown in dropdowns)
    10.     {
    11.         dropdown.options.Clear();
    12.         dropdown.AddOptions(lst);
    13.     }
    14.     return lst;
    15. }
    This way the function intuitively speaks to the idea that the list of strings is a result, not an input.

    And if you really need that dictionary to be built... then like, create a 2nd function that builds that dictionary.

    Code (csharp):
    1. void MapList<T>(IReadOnlyList<T> lst, Dictionary<int, T> dict)
    2. {
    3.     for (int i = 0; i < lst.Count; i++) dict[i] = lst[i];
    4. }
    Or better:
    Code (csharp):
    1. Dictionary<int, T> MapList<T>(IReadOnlyList<T> lst)
    2. {
    3.     var dict= new Dictionary<int, T>();
    4.     for (int i = 0; i < lst.Count; i++) dict[i] = lst[i];
    5.     return dict;
    6. }
    Then just call it after you call FillDropDownsAndActivities.

    I still don't see the need for the dictionary... but at least this way it's not FillDropDownsAndActivities job to be creating it.
     
    SprayNpraY likes this.
  10. Neto_Kokku

    Neto_Kokku

    Joined:
    Feb 15, 2018
    Posts:
    1,751
    To try to explain in a simpler way: you cannot add an
    Activity
    to a
    List<RewardingActivity>
    . This check is performed at compile time (not during execution time) which is why the compiler will complain when you try to pass a
    List<RewardingActivity>
    into a function which expects a
    List<Activity>
    .
     
  11. SprayNpraY

    SprayNpraY

    Joined:
    Oct 2, 2014
    Posts:
    156
    Thanks, sorry for overlooking it I was getting confused looking through all of the info on contravariance etc and I didn't fully get your suggestion.

    I've just tried it and your solution has worked for both the RewardingActivity and PunishmentingActivity.

    First, thank you very much for your detailed responses. I'm struggling to understand them but will come back to them in the future when I understand contra and covariance better to help further deepen my understanding.

    As for why I'm using a method that I've made awkward for myself, I already thought there must be a much better/easier way of doing this. I'm quite inexperienced so it was the only way at this moment in time I could think of solving the issue of:
    Getting a group of dropdown menus, to be filled based on what I put into a list while also the selected drop downs can be used to add based on the corresponding value total based on what the user picks.

    I'll probably totally change the design in the future though.

    Hope you recover soon, and I don't think you're rambling despite that I'm struggling to fully grasp what you're saying but I am still finding it useful.

    To respond to everything else you've said because there's a lot of individual areas that are worthy of a response but to avoid making my response huge.

    Basically the method isn't just to fill the drop downs, but also to expand on what I've said above. Make it so calculations can be done based on the selected drop downs, and the only way I could think of storing the values of the selected drop downs to also be retrieved is through a dictionary.
    Maybe you're the one physically ill but after looking at my code again I think it feels like I've had a fever dream because I've just realized I can access the same value from the list instead of needing the dictionary to also include it, so I removed the use of the dictionary.

    It may have been because of thinking I had to find the value based on the string, then I realized I could use the int value of the selected drop down value, only my unconscious and God knows at this point :)

    I've now changed to using your suggestion except for the return type being void:
    Code (CSharp):
    1.     void FillDropDownsAndActivities(IEnumerable<Activity> activities, IEnumerable<TMP_Dropdown> dropdowns)
    2.     {
    3.         var lst = new List<string>();
    4.         foreach (var a in activities)
    5.         {
    6.             lst.Add($"{a.ActivityName}" +
    7.                 $" RP:{a.Value}");
    8.         }
    9.         foreach (var dropdown in dropdowns)
    10.         {
    11.             dropdown.options.Clear();
    12.             dropdown.AddOptions(lst);
    13.         }
    14.     }
    And it works, thanks again.
     
    lordofduct likes this.