CODEWITHSUNDEEP
c#
Taelia
Taelia

Reputation: 601

Looking for better approach to deal with lots of parameters

I have implemented a form of Steering behavior for a game, and per use of an editor, a designer should be able to choose for (currently) seven parameters whether or not they should be 0, random or a set amount, and in case of the latter two, in which range random should fall or which set amount the variable should get. The settings will be saved in a file, and have to be able to be loaded in at any time.

What I ended up with is a painful to look at constructor and a whole, whole lot of fields and properties. I am looking for a more intelligent approach to solve this problem, especially when adding more parameters later.

public enum SteerType
{
    None,
    Random,
    Variable
}

public IdleSteering(SteerType rotationOnIntervalType, int rotationOnInterval, 
                    SteerType rotationOnCollisionType, int rotationOnCollision,
                    SteerType breakIntervalType, int breakInterval,  
                    SteerType breakTimeType, int breakTime, 
                    SteerType rotationOnBreakType, int rotationOnBreak, 
                    SteerType rangeFromSpawnType, int rangeFromSpawn, 
                    SteerType speedType, int speed)
    {
        _rotationOnIntervalType = rotationOnIntervalType;
        _rotationOnInterval = rotationOnInterval;

        _rotationOnCollisionType = rotationOnCollisionType;
        _rotationOnCollision = rotationOnCollision;

        <...> //And this five more times
    }

And the following chunk for every single parameter, so seven times as well.

private SteerType _rotationOnIntervalType;
    private float _randomRotationOnInterval;
    private float _rotationOnInterval;
    public float RotationOnInterval
    {
        get
        {
            switch (_rotationOnIntervalType)
            {
                case SteerType.None:
                    return 0;
                case SteerType.Random:
                    if (_randomRotationOnInterval.Equals(0))
                        _randomRotationOnInterval =
                            MathHelper.ToRadians((float)(-Static.Random.NextDouble() + Static.Random.NextDouble()) * _rotationOnInterval);
                    return _randomRotationOnInterval;
                case SteerType.Variable:
                    return _rotationOnInterval;
            }
            return 0;
        }
    }

Upvotes: 1

Views: 92

Answers (5)

Val Bakhtin
Val Bakhtin

Reputation: 1464

I'd make another Rotation with SteerType and RotationValue (and your getter, virtual if needed), and will default parameters in constructor like this (.NET 4.0 req'ed):

public IdleSteering(Rotation interval=null, Rotation collission=null)
    {

        _rotationOnIntervalType = rotationOnIntervalType??new Rotation(...);
        _rotationOnInterval = rotationOnInterval??new Rotation(...);
        ...
    }

then you would just call:

var idleSteering = new IdleSteering();

of course if properties can be defaulted...

Upvotes: 0

Andr
Andr

Reputation: 627

It looks like you have a collection, of potentially indefinite length, consisting of different parameters.

Why not make it with a dictionary, like so:

public IdleSteering(IDictionary<SteerType,int> steerSettings)

and then go through the dictionary keys to set and get a values.

?!?

Upvotes: 0

Adam Robinson
Adam Robinson

Reputation: 185643

The most obvious solution would be to look for patterns in your parameters. What I see is that you have combinations of SteerType along with an int that represents a speed. You then have this combination several times, once each for several different states.

Given that, you could combine these two things into a single class:

public class SteerSettings
{
    public SteerType Type { get; private set; }
    public int Interval { get; private set; }

    public SteerSettings(SteerType type, int interval)
    {
        Type = type;
        Interval = interval;
    }
}

Then change your constructor to look like this:

public IdleSteering(SteerSettings rotationOn, SteerSettings collision, 
                    SteerSettings break,  SteerSettings breakTime, 
                    SteerSettings rotationOnBreak, SteerSettings rangeFromSpawn, 
                    SteerSettings speed)
    {
        ...
    }

A more aggressive approach would be to take that and create a new enum that represents your various states and use that in a Dictionary to specify the various settings for each state.

public enum SteerState
{
    RotationOn,
    Collision,
    Break,
    BreakTime,
    RotationOnBreak,
    RangeFromSpawn,
    Speed
}

public IdleSteering(IDictionary<SteerState, SteerSettings> settings)
{
    ...
}

Upvotes: 2

p.campbell
p.campbell

Reputation: 100577

Suggest creating a class to hold the 2/paired values, and then group into a collection:

public class Steering
{
    public SteerType SteerType{get;set}
    public int Amount{get;set}
}

List<Steering> userSteering = new List<Steering>
  {
     //initialize your list
  };

//pass that list into a constructor.

Over in your IdleSteering class:

private List<Steering> steeringValues;

public IdleSteering(List<Steering> userSteering)
{
    steeringValues = userSteering;
}

//we can now use LINQ to find any particular steering value.
int braking = steeringValues.SingleOrDefault(x=>x.SteerType==Random).Amount;

Pass and use that collection between all your methods.

Upvotes: 3

Oded
Oded

Reputation: 499012

Use a settings object - refactor these parameters into a class and pass that class in. You can add additional parameters without an issue.

This is called a parmeter object refactoring and has been used in several places in the BCL - the ProcessStartInfo class being one.

An added bonus to this approach is that you can serialize and deserialize the settings separately from the class using them.

Upvotes: 3

Related Questions