0

I'm using a static class as some kind of library (maybe not the best idea ever but I don't really know what I'm doing...) and I have some fields with objects so I can access them easily by name. But I still need to have an array that I can use a method to search through.

But when I call that method GetTypeByName("Iron Mine") it should return look through the array and return the ironMine object, but instead it just screams "Object reference not set to an instance of an object"... I'm clearly missing something about static arrays or arrays in general, I'm not that used to them yet...

public static class IndustryTypes
{
    public static IndustryType[] allTypes = //This is that array that seems to mess things up...
    {
        ironMine, coalMine, aluminiumMine, copperMine, uraniumMine, goldMine,
        quarry, oilWell,
        farm
    };

    public static IndustryType noType;
    public static IndustryType ironMine, coalMine, aluminiumMine, copperMine, uraniumMine, goldMine;
    public static IndustryType quarry, oilWell;
    public static IndustryType farm;

    static IndustryTypes()
    {
        noType = new IndustryType("noType", 0, Color.Red);
        ironMine = new IndustryType("Iron Mine", 50000, Game.color_mine);
        coalMine = new IndustryType("Coal Mine", 40000, Game.color_mine);
        aluminiumMine = new IndustryType("Aluminium Mine", 100000, Game.color_mine);
        copperMine = new IndustryType("Copper Mine", 55000, Game.color_mine);
        uraniumMine = new IndustryType("Uranium Mine", 150000, Game.color_mine);
        goldMine = new IndustryType("Gold Mine", 125000, Game.color_mine);

        quarry = new IndustryType("Quarry", 25000, Game.color_mine);
        oilWell = new IndustryType("Oil Well", 110000, Game.color_oil);
        farm = new IndustryType("Farm", 10000, Game.color_farm);

    }

    public static IndustryType GetTypeByName(string name)
    {
        for (int i = 0; i < allTypes.Length; i++)
        {
            if (name == allTypes[i].name) //This says "Object reference not set to an instance of an object" or something like that...
            {
                return allTypes[i];
            }
        }
        return noType;
    }
}

This is all in a static class called "IndustryTypes", so I don't need to instantiate it.

The class "IndustryType" is a non-static class.

Ask if you don't understand what I mean... I don't really know myself but I'll try! :D

Here's the "IndustryType"-class:

public class IndustryType
{
    public string name;
    public int cost;
    public Color color;
    public List<Resource> components;
    public List<Resource> products;

    public IndustryType(string _name, int _cost, Color _color)
    {
        name = _name;
        cost = _cost;
        color = _color;
    }
}

Thanks a lot for taking your time!

NineBerry
  • 20,173
  • 3
  • 50
  • 78
  • 1
    Your array is being constructed with null references because of your collection initializer. Move the array construction into the static constructor. – pinkfloydx33 Feb 25 '17 at 17:19
  • @Svek I didn't copy everything, if you mean the first line, that's the array – Anton J Håkansson Feb 25 '17 at 17:20
  • @AntonJHåkansson take a look at my answer, hard to describe in comments. – Svek Feb 25 '17 at 17:21
  • Actually this is maybe a better duplicate http://stackoverflow.com/q/8285168/491907 – pinkfloydx33 Feb 25 '17 at 17:26
  • @pinkfloydx33 ok... how would I do that? I tried static IndustryType[] allTypes; but then it wouldn't let me do: allTypes = { blah blah }; – Anton J Håkansson Feb 25 '17 at 17:29
  • 1
    You'd have to use `allTypes = new IndustryType[] { blah blah };` – NineBerry Feb 25 '17 at 17:31
  • See my updated answer – pinkfloydx33 Feb 25 '17 at 17:33
  • @NineBerry So then I'd have to make it public to be able to access it outside the constructor? – Anton J Håkansson Feb 25 '17 at 17:33
  • Also you should rethink all those public fields that should probably be properties presumably without setters – pinkfloydx33 Feb 25 '17 at 17:34
  • @AntonJHåkansson It's already public in the question ;-) But no, it can be private, you can access private static fields from the static constructor. – NineBerry Feb 25 '17 at 17:35
  • 1
    Don't use `static` unless you have a good reason to. See [When to use static classes in C#](http://stackoverflow.com/questions/241339/when-to-use-static-classes-in-c-sharp). – Mike Lowery Feb 25 '17 at 17:42
  • @DiskCrashser `static` (whether properties or clsses) is absolutely fine and often times necessary. Though in this case he should move the static fields (preferably as properties) so that they are members of the `IndustryType` (not plural) class – pinkfloydx33 Feb 25 '17 at 17:49
  • @pinkfloydx33 I'm not proposing you never use them but I don't see a valid reason for using them in this case. And because he did, it caused confusion with respect to static initialization. I've inherited projects where nearly all classes were static and that was simply not good for a number of reasons. When necessary, a singleton is often a better choice. – Mike Lowery Feb 25 '17 at 17:56
  • 1
    I agree he should remove this static class. But he should keep the static fields (encapsulated by properties) and move them into the non pluarlized class where they belong. A static property named AllTypes surely makes sense (though better implemented perhaps with a list that has items added to it by the class' instance constructor). Singletons are one of the most oftenly despised anti patterns. They have their uses but that's a broad statement. Static classes are useful for utility classes (like the IO classes) but also for caching via nested static classes. – pinkfloydx33 Feb 25 '17 at 18:01

2 Answers2

2

You are defining your array as an array of null references. Move the array construction to the end of your static constructor.

public static IndustryType[] allTypes;

public static IndustryType noType; 
public static IndustryType ironMine, coalMine, aluminiumMine, copperMine, uraniumMine, goldMine; 
public static IndustryType quarry, oilWell; 
public static IndustryType farm;

static IndustryTypes() {
    noType = new IndustryType("noType", 0, Color.Red);
    ironMine = new IndustryType("Iron Mine", 50000, Game.color_mine);
    coalMine = new IndustryType("Coal Mine", 40000, Game.color_mine);
    aluminiumMine = new IndustryType("Aluminium Mine", 100000, Game.color_mine);
    copperMine = new IndustryType("Copper Mine", 55000, Game.color_mine);
    uraniumMine = new IndustryType("Uranium Mine", 150000, Game.color_mine);
    goldMine = new IndustryType("Gold Mine", 125000, Game.color_mine);

    quarry = new IndustryType("Quarry", 25000, Game.color_mine);
    oilWell = new IndustryType("Oil Well", 110000, Game.color_oil);
    farm = new IndustryType("Farm", 10000, Game.color_farm);


   allTypes = new [] {
    ironMine, coalMine, aluminiumMine, copperMine, uraniumMine, goldMine,
    quarry, oilWell, farm }; 
}

If you are never going to directly access those public fields, ie via IndustryTypes.goldMine then you could just remove them and assign them all at once to the array without the fields.

allTypes=new[] { 
          new IndustryType("noType", 0, Color.Red), 
          new IndustryType("Iron Mine", 50000, Game.color_mine), 
          new IndustryType("Coal Mine", 40000, Game.color_mine), 
          new IndustryType("Aluminium Mine", 100000, Game.color_mine), 
          new IndustryType("Copper Mine", 55000, Game.color_mine), 
          new IndustryType("Uranium Mine", 150000, Game.color_mine), 
          new IndustryType("Gold Mine", 125000, Game.color_mine), 
          new IndustryType("Quarry", 25000, Game.color_mine), 
          new IndustryType("Oil Well", 110000, Game.color_oil), 
          new IndustryType("Farm", 10000, Game.color_farm) 
};

I would recommend using public properties over fields if you can and moving this all into the IndustryType class (not plural)

Note that your array is exposed. It is not readonly and its elements can be changed. You'd be better off encapsulating it behind a property exposed as one of the IReadOnlyXXX interfaces. Otherwise someone can just do IndustryTypes.allTypes[0]=null; (or possibly worse, setting it to a new IndustryType instance) without your knowledge/permission.

pinkfloydx33
  • 9,666
  • 3
  • 34
  • 51
2

The static constructor of the IndustryTypes class is executed after all the static fields are initialized. So, at the moment, when this is executed:

public static IndustryType[] allTypes = //This is that array that seems to mess things up...
{
    ironMine, coalMine, aluminiumMine, copperMine, uraniumMine, goldMine,
    quarry, oilWell,
    farm
};

all the fields like ironMine etc. have still not been initialized and point to null.

Solution: Move the initialization of the fields from the static constructor to the declaration itself. Also pay attention, that the initializers are executed in textual order, so you must declare the array last after all the other fields.

public static class IndustryTypes
{
    public static IndustryType noType = new IndustryType("noType", 0, Color.Red);
    public static IndustryType ironMine = new IndustryType("Iron Mine", 50000, Game.color_mine);
    public static IndustryType coalMine = new IndustryType("Coal Mine", 40000, Game.color_mine);
    public static IndustryType aluminiumMine = new IndustryType("Aluminium Mine", 100000, Game.color_mine);
    public static IndustryType copperMine = new IndustryType("Copper Mine", 55000, Game.color_mine);
    public static IndustryType uraniumMine = new IndustryType("Uranium Mine", 150000, Game.color_mine);
    public static IndustryType goldMine = new IndustryType("Gold Mine", 125000, Game.color_mine);
    public static IndustryType quarry = new IndustryType("Quarry", 25000, Game.color_mine);
    public static IndustryType oilWell = new IndustryType("Oil Well", 110000, Game.color_oil);
    public static IndustryType farm = new IndustryType("Farm", 10000, Game.color_farm);

    public static IndustryType[] allTypes = {
        IndustryTypes.ironMine,
        IndustryTypes.coalMine,
        IndustryTypes.aluminiumMine,
        IndustryTypes.copperMine,
        IndustryTypes.uraniumMine,
        IndustryTypes.goldMine,
        IndustryTypes.quarry,
        IndustryTypes.oilWell,
        IndustryTypes.farm
    };
}
NineBerry
  • 20,173
  • 3
  • 50
  • 78