Compiler Warnings vs Code Analysis

public class Team
{
      private IList<Athlete> athletes;
      ...
}
Compiler Warning: Field Team.athletes is never assigned to, and will always have its default value null.
( its assigned to in my mapper layer )
 
public class Team
{
      public Team()
      {
            athletes = null;
      }
      private IList<Athlete> athletes;
      ...
}
Code Analysis Warning: Team.Team() initializes field entries of type System.Collections.Generic.IList<Athlete> to null. Remove this initialization as it will be done automatically by the runtime.
 
What am I doing wrong here?  Initializing athletes in the Team constructor to a new List<Athlete>() seems like a waste since its being set/initialized in my factory.
 
Update: Kicks to Stuart for "setting" me right.
 

6 Comments

  • If it were being initialized in your factory, you wouldn't be getting the compiler warning.



    Since it's a private field, if you're talking about a factory *class* then it couldn't access the athletes field to assign to it, so I assume you're talking about a static factory method like Team.Create(). If that method assigns a value to the athletes field on the newly created Team instance, you wouldn't get the warning, because it *is* assigned to.



    What does your factory code look like?



    Basically, both the warnings are right; the first version is what you should write, and something's wrong elsewhere in your code. Assigning to null is pointless, but the first warning's telling you that not only are you not initializing it, you're never assigning *anything* to it later either.

  • My factory is using an ormapper and reflection to assign the attributes. However, your post made me realize I didnt have a public setter for that attribute. The warnings are gone now.

  • Seems kind of strange that your public and publically constructable class is relying on other objects to instantiate it's fields.



    For example, calling code would get exceptions if it tried to do something obvious like



    Team t = new Team();

    t.Athletes.Add(..); //error here



    In other words, the class probably shouldn't have a public constructor.

  • Matt you are right; I should instantiate Athletes in the Team contructor in the case of Team being instantiated outside of my factory. Is there a way to keep a class from being instantiated outside of a factory? Is that &quot;good&quot; practice in the first place? ...still learning.

  • Travis,



    Just to comment on your question &quot;Is there a way to keep a class from being instantiated outside of a factory?&quot;.



    Simply make the constructor for your class private.



    Not sure what your factory class looks like at the moment (might be worth posting a skeleton), but you could say:



    public class Team

    {

    private Team()

    {}



    private List&lt;Athlete&gt; _athletes;

    public List&lt;Athlete&gt; Athletes

    {

    // usual getter and setter, left out for brevity

    }



    public static Team CreateTeam()

    {

    return new Team();

    }

    public static Team CreateTeam(List&lt;Athlete&gt; athletes)

    {

    Team t = CreateTeam();

    t._athletes = athletes;

    return t;

    }

    }



    Again - just an illustrative sample. You can't instantiate a Team any other way than through one of the two factory methods.

  • In addition to my previous post, I guess you could modify the first factory method to instantiate the Athletes list.



    It all depends on the pre-conditions.

Comments have been disabled for this content.