Method overloading

Unity Engine
1

hello,

Im currently in the process of writing a sfx player which I will use to play my custom sfx’s. Each sfx has their own set of audioClips which can be played at random or in order. The sfx manager creates a pool of AudioSources connected to these sfx’s but the user can optionally provide an AudioSource when playing the sfx.

So I started using method overloading for the Play() method. I was wondering if this could be simplified and if this looks reasonable or that im overcomplicating things:

        public void Play(string name)
        {
            Play(name, Style.None, false);
        }

        public void Play(string name, Style style)
        {
            Play(name, style, false);
        }

        public void Play(string name, bool fadeIn)
        {
            Play(name, Style.None, fadeIn);
        }

        public void Play(string name, Style style, bool fadeIn)
        {
            //DO ALL LOGIC WITH DEFAULT AUDIOSOURCE
        }

        public void Play(string name, AudioSource source)
        {
            Play(name, source, Style.None, false);
        }

        public void Play(string name, AudioSource source, Style style)
        {
            Play(name, source, style, false);
        }

        public void Play(string name, AudioSource source, bool fadeIn)
        {
            Play(name, source, Style.None, fadeIn);
        }

        public void Play(string name, AudioSource source, Style style, bool fadeIn)
        {
            //DO ALL LOGIC WITH PROVIDED AUDIOSOURCE
        }

Would it okay to add a null as parameter so i onyl have to deal once with the logic?

     public void Play(string name, Style style, bool fadeIn)
        {
            Play(name, null, style, fadeIn);
        }
2

Yes this can be simplified into a single method like so, it will generate all the overloads for you

public void Play(string name, AudioSource source = null, Style style = Style.None, bool fadeIn = false)
{
    if (source == null)
    {
        //DO ALL LOGIC WITH DEFAULT AUDIOSOURCE
    }
    else
    {
        //DO ALL LOGIC WITH PROVIDED AUDIOSOURCE
    }
}
1 Like
3

Instead of splitting/duplicating the logic with the if/else, have the IF section assign the default source, then run the logic.

public void Play(string name, AudioSource source = null, Style style = Style.None, bool fadeIn = false)
{
    if(source == null)
    {
        source = // get & assign default audio source
    }

    // do all logic on "source"
}
1 Like
4

The only downside to that approach is crapping up your method calls with nulls etc just to get to the argument you care about. So if I wanted to play some clip with a fade in I’d have to write

Play("foo", null, Style.None, true);

You can remedy this by ordering the arguments based on how often you plan on not using the default value. So if you know you’ll mix a lot of fades with non-fades you can put it first so that you can omit the other arguments

Play("foo", true); // default source and style by omitting

Now - I’m not a huge fan of boolean arguments like that because it isn’t readily apparent what true represents in this case just by looking at that line of code. But that’s personal preference more than anything else.

1 Like
5

I definitely agree that it hurts readability.
Theres always named arguments :wink:

2 Likes
6

My thoughts exactly, you can skip over the optional parameters you don’t care about by naming the ones you do care about:

Play("foo", fadeIn: true);
1 Like
7

I’d like to know a little more about your problem. Are the decisions about the parameters of a particular sound effect (e.g., whether or not to fade in, which style to use, et cetera) really the responsibility of the caller or are they traits of particular sound effects?

EDIT: (recurring traits, I meant)