Is there a way to make this shorter?

Question

So I'm quite new to programming in general. I'm currently working on a terrain generation program, everything is going great except for this:

    public static class Desert
    {
        public const int iChance = 15;
        public static int chance = iChance;
        public static int chancepoint = 0;
        public const int octaves = 4;
        public const int lengthMin = 60;
        public const int lengthMax = 90;
        public const float scaleMin = 250;
        public const float scaleMax = 350;
        public const float persistenceMin = 0.5f;
        public const float persistenceMax = 0.9f;
        public const pType ptype = pType.Lowland;
        public const bTag[] tags = { bTag.desert };
    }
    public static class Meadow
    {
        public const int iChance = 45;
        public static int chance = iChance;
        public static int chancepoint = 0;
        public const int octaves = 4;
        public const int lengthMin = 45;
        public const int lengthMax = 70;
        public const float scaleMin = 200;
        public const float scaleMax = 470;
        public const float persistenceMin = 0.35f;
        public const float persistenceMax = 0.70f;
        public const pType ptype = pType.noAbs;
        public const bTag[] tags = { bTag.lush };
    }

These are the properties for each different type of 'Biome'.

I currently have about 7 of these and they're all exactly the same except for the values of each field.

Is there a way that I can shorten the code? I looked into inheritance but I ended up with errors and I got a little confused. ><

It would be brilliant if all I had to write was:

public static class Desert
    {
        iChance = 15;
        chance = iChance;
        chancepoint = 0;
        octaves = 4;
        lengthMin = 60;
        lengthMax = 90;
        scaleMin = 250;
        scaleMax = 350;
        persistenceMin = 0.5f;
        persistenceMax = 0.9f;
        ptype = pType.Lowland;
        strongTags = { bTag.desert };
    }

Thanks in advance.

Oh, and sorry about the nubness of the question, you would probably scream at how terrible my code was if you saw the rest of the program. XD

EDIT: It's probably wise to tell you that I NEVER change the stuff within the class again with the exception of the value of 'chance'.

Remove the 'static' keyword and put all those in the constructor — MikeSW, May 20 '13 at 08:14
You are confusing a class with its instance. Don't declare 7 classes, declare 1 and instantiate it for each biom. Then go out and buy a book about programming concepts :-) — Ondrej Tucny, May 20 '13 at 08:17
Like @OndrejTucny said, it seems like what you need is one class but many instances for different types of Biome. Then you could automate it even more to read the values from file, instantiate them automatically, like through de-serialization or something. — Jarek, May 20 '13 at 08:31

joe · Answer 1 · 2013-05-20T08:34:28.767

Instead of using a static class, you can use a non-static class.

public class Biome {
    // Instance fields with default values
    public int iChance = 15;
    public int chance = iChance;
    public int chancepoint = 0;
    public int octaves = 4;
    public int lengthMin = 60;
    public int lengthMax = 90;
    public float scaleMin = 250;
    public float scaleMax = 350;
    public float persistenceMin = 0.5f;
    public float persistenceMax = 0.9f;
    public pType ptype = pType.Lowland;
    public bTag[] tags = { bTag.desert };
}

Here use the constructor for initializing:

public Biome(int iChance, int chance, int chancepoint, int octaves, public int lengthMin, int lengthMax, float scaleMin, float scaleMax, float persistenceMin, float persistenceMax,pType ptype, bTag[] tags) {
    // init fields here
}

Then call the constructor:

Biome bimoe = new Biome(15, iChance, 0, 4, 60, 90, 250, 350, 0.5f, 0.9f, pType.Lowland, { bTag.desert });

With this it's difficult to see which parameter goes to which field, but it's much shorter.

If the fields must be read-only, you can make properties with only a public get and no set accessor. Example:

public Chance { get { return chance; } }

In this case make the fields private:

private int chance = iChance;

(Personally, for such a scenario, i would put all the data in a file)

Marc Gravell · Answer 2 · 2013-05-20T08:27:49.907

The following would be shorter:

public const int iChance = 15, octaves = 4, lengthMin = 60, lengthMax = 90;
public const float scaleMin = 250, scaleMax = 350, persistenceMin = 0.5f,
                   persistenceMax = 0.9f;
public static int chance = iChance, chancepoint = 0;

However... these really don't look like things that should be static fields, or quite possibly not even const. They look like things that should be instance properties. Maybe something like:

public class Terrain {
   public int Chance {get;private set;}
   public int LengthMin {get;private set;}
   // ...
   private Terrain(int chance, int lengthMin, ...) {
       Chance = chance;
       LengthMin = lengthMin;
       // ...
   }
   private static readonly Terrain
       desert = new Terrain(45, 45, ...),
       meadow = new Terrain(15, 60, ...),
       ...;
   public static Terrain Desert { get { return desert;}}
   public static Terrain Meadow { get { return meadow;}}
}

Okay, that makes sense but I never ever change the stuff within the class once I create them and performance is rather important. Would this affect performance? — AwesomePerson, May 20 '13 at 08:31
@AwesomePerson tricky one. Constants have the advantage that they are "burned in" into the caller (it is never ldfld etc - it is a raw load-value). How much that matters depends a lot on the scenario (you would need to profile). But the syntax in the first sample here (multiple values per declaration) is at least less verbose! — Marc Gravell, May 20 '13 at 08:49

score 1 · Answer 3 · answered May 20 '13 at 08:28

1

I don't know much about terrain generation programs, but you should store your data in a database. Then create classes to map that data to your application. I recommend you to lookup "Data structures" and see which one fits your application the best.

answered May 20 '13 at 08:28

Nagy Ladislau Radu

118
12

score 0 · Answer 4 · answered May 20 '13 at 08:19

0

It's better to use only one class without inheritance, or even structure. Desert, Meadow and so on are not classes logically, it's have to be objects (maybe constants).

answered May 20 '13 at 08:19

x2.

9,554
6
41
62

score 0 · Answer 5 · answered May 20 '13 at 08:26

What you could do is use a single class called Terrain and Initialise this multiple times using a static constructor:

   public class Terrain
    {
            public int IChance { get; private set; }
    public int Chancepoint { get; private set; }
    public int Octaves { get; private set; }
    public int LengthMin { get; private set; }
    public int LengthMax { get; private set; }
    public float ScaleMin { get; private set; }
    public float ScaleMax { get; private set; }
    public float PersistenceMin { get; private set; }
    public float PersistenceMax { get; private set; }
    public pType Ptype { get; private set; }
    public bTag[] Tags { get; private set; }

        public static Terrain Desert()
        {
            return new Terrain
                {
                    IChance = 15,
                    Chancepoint = 0,
                    Octaves = 4,
                    LengthMin = 60,
                    LengthMax = 90,
                    ScaleMin = 250,
                    ScaleMax = 350,
                    PersistenceMin = 0.5f,
                    PersistenceMax = 0.9f,
                    Ptype = pType.Lowland,
                    Tags = new bTag[] {bTag.Desert}
                };
        }
}

score 0 · Answer 6 · answered May 20 '13 at 08:26

joe's answer is good, but the constructor call has far too many unnamed parameters - what does the 350 mean?

This is an ideal candidate for data driven design.

Rather than define all the Biome types in the code, put all the data for the Biome types into a file and read the file at run time. The C# language has a lot of stuff to help you do this, the key word to search for is Serialisation (and here's a link to MSDN about it).

The big advantage is that you can change the data values without needing to recompile the code.

The disadvantage is that is takes a lot more code to define the first instance, but after that, you can easily create as many instances as you want.

score 0 · Answer 7 · answered May 20 '13 at 08:29

You could do something like declaring an abstract class like this and then inherting from it:

    public abstract class Terrain
    {
        public int iChance;
        public int chance;
        public int chancepoint;
        public int octaves;
        public int lengthMin;
        public int lengthMax;
        public float scaleMin;
        public float scaleMax;
        public float persistenceMin;
        public float persistenceMax;
        public pType ptype;
        public Tag[] strongTags;
    }

    public class Desert : Terrain
    {

    }

    public enum pType
    {
        Desert = 1,
        LowLand = 2
    }

    public enum Tag
    {
        desert = 1,
        lush = 2
    }

You can then instantiate desert like :

var desert = new Desert()
            {
                iChance = 15
                ,chance = 15
                ,chancepoint = 0
                ,octaves = 4
                ,lengthMin = 60
                ,lengthMax = 90
                ,scaleMin = 250
                ,scaleMax = 350
                ,persistenceMin = 0.5f
                ,persistenceMax = 0.9f 
                ,ptype = pType.Desert
                ,strongTags = new Tag[]{Tag.desert}
            };

Dimitar Dimitrov · Answer 8 · 2013-05-20T08:36:15.973

Not sure what your exact requirements are but wouldn't this be a better approach:

public abstract class BiomeBase
{
    public int Chance      { get; set; }
    public int Chancepoint { get; set; }
    public int Octaves     { get; set; }
    // you get the idea ...
}

Then you have Desert and Meadow inheriting:

public class Desert : BiomeBase
{
    // everything is inherited ...
    // you can also add your own properties meant for Desert only (if needed)
}

public class Meadow : BiomeBase
{
    // everything is inherited ...
}

Now Desert has everything Biome has and you can use it like this:

var desert = new Desert
{
    Chance = 5,
    Octaves = 1, 
    /// etc
};

Holly cow ... how many answers ... I don't even :))) Sorry if repeated. — Dimitar Dimitrov, May 20 '13 at 08:31

score -2 · Answer 9 · edited May 23 '17 at 12:23

Firstly you cant do inheritance on static classes. So you would have to start using instances.

Secondly you would use inheritance if you wanted to extend the object. So for instance if you wanted to add a new property "bool HasScorpions" onto Desert but not on Meadow.

Since your using the same properties but want to use different values I personally use an Interface. This way you can make the properties readonly ect while still easily setting the values.

public interface Terrain
{
    int iChance = {get { return 15; private set; } ..and repeat.
    int chance = iChance;
    int chancepoint = 0;
    int octaves = 4;
    int lengthMin = 60;
    int lengthMax = 90;
    float scaleMin = 250;
    float scaleMax = 350;
    float persistenceMin = 0.5f;
    float persistenceMax = 0.9f;
    pType ptype = pType.Lowland;
    bTag[] tags = { bTag.desert };
}

Is there a way to make this shorter?

9 Answers9