Discussion Replace StartCoroutine(string) with StartCoroutine(Delegate)?

Is this good practice? It reduces hard coding.

Code (CSharp):

1. StartCoroutine(Delegate method)
2. {
3.     StartCoroutine(method.Method.Name);
4. }

 

I fail to see how this reduces any hardcoding. What this does do is abandon compile time safety and allocate extra memory (presumably) when used.

 

What's stopping you from using

StartCoroutine(IEnumerator routine)

? No hardcoding required.

 

I'm curious what you actually talk about What does your actual usecase of this (horrible) implementation looks like and what's your alternative? You shouldn't use the string version anyways. It's very limited as it can only call methods that are declared in the same class, you can't really pass any arguments and the string binding is slower.

So what's your exact usecase here? Do you need to stop the coroutine? That's also generally not a good design as from the coroutines perspective you never know when you will be terminated. That's why the coroutine should decide when it's finished. Yes there are some cases where it makes sense to stop a coroutine from the outside, however that can be done without using the string binding. You should have to store the coroutine instance in a variable.

If it's about restarting the same coroutine, this can also be done quite easily by using IEnumerable instead of IEnumerator. When you want to start a coroutine you simply call GetEnumerator on the IEnumerable. That way even arguments are stored and re-used and the caller / starter of the coroutine does not need to know anything about the coroutine.

So it would be great if you could clarify your usecase and setup.

 

StartCoroutine does not accept and argument type of Delegate, it accepts an IEnumerator argument.

 

If you had to use the string-based overload, you could use the nameof expression instead of a delegate for the same benefits:

Code (CSharp):

1. StartCoroutine(nameof(MyMethod));

But yeah, there is also the IEnumerator overload, which should always be used instead:

Code (CSharp):

1. StartCoroutine(MyMethod());

 

Thanks for the replies, I'm a beginner and I'm simply confused about methods that accept strings, as programming tutorials usually tell us that hardcoding should be avoided, and as others have mentioned,

Code (CSharp):

1. nameof(Method)

might be an acceptable solution, but if the `StartCoroutine` method itself accepts the `Delegate` parameter, perhaps it could be more concise?

 

This makes no sense. StartCoroutine expects an object as parameter. A coroutine is not a "method". The method you create is a generator method that creates a state machine when called. This statemachine object needs to be passed to StartCoroutine. So delegates just makes no sense here. You may want to have a look at my coroutine crash course.

 

It would only be two less characters compared to calling the IEnumerator-based overload though, which is not enough of a difference in my opinion to affect legibility.

Code (CSharp):

1. StartCoroutine(MyMethod); // <- Delegate; a reference to MyMethod

vs

Code (CSharp):

1. StartCoroutine(MyMethod()); // <- IEnumerator; the return value of MyMethod

But on the flip side the code becomes more error-prone, because you could pass a reference to any method regardless of its return type, parameter list and overload count, and the compiler wouldn't be able to warn you about any mistakes you make.

Code (CSharp):

1. void Test() => StartCoroutine(MyMethod); // <- compiles just fine
2. void Test2() => StartCoroutine(()=>MyMethod()); // <- compiles just fine
3.  
4. void MyMethod() => Debug.Log("This is not a coroutine.");
5. void MyMethod(string parameter) => Debug.Log("This also needs an argument");

If there was no IEnumerator-based StartCoroutine, then I would agree that your custom overload would be a small improvement over using the

nameof

expression in terms of legibility.

In terms of performance there could potentially be some negative side effects though, as creating the delegate that is passed to the method could result in garbage being generated with each call, unless the compiler is able to optimize it by caching and reusing a single delegate.

And then there's also the risk that using a custom StartCoroutine variant like this in a codebase could end up being more of a source of confusion rather than clarity - at least initially - because everybody who reads the code would be expecting StartCoroutine to only accept a string or a coroutine, so they would likely be left wondering how in the world the code even compiles.

 

I love the WTF/s concept because it's just fact. When you read someone else's code you usually find they do things quite different to your own style. That's why you usually have several WTF moments when trying to understand the code. The WTF/s is really a good metric

Though I'm still interested in the usecase and whats the reason why he choose to use the string version in the first place. Even the craziest things could have a relatively good justification. Though in many cases there are better approaches to achieve the same goal.

 

I think there is no particular use case, but the OP is just interested in general about strategies for avoiding magic strings in code. He came up with the hypothetical idea of referring to methods using a delegate instead of the name of method, and is wondering whether or not that sounds like a good approach or not:

With the Unity API containing stuff like

StartCoroutine(string)

,

InvokeRepeating(string, ...)

,

GameObject.FindWithTag(string)

,

Animator.SetTrigger(string)

(and even

GetComponent(string)

used to be a thing ) many tutorials out there will probably be littered with examples using magic strings...

 

Yes, these magic strings amaze me. And I think Unity/Visual Studio could enhance the co-development experience by giving these magic strings IntelliSense, and rename tracking.

 

...what?

Magic strings are bad practice generally. Their use should not be promoted.

The only 'magic strings' you ought to be using in Unity on the regular are Unity's 'magic methods' (Awake, Start, Update, etc), which already have intellisense suppose.

 

That's why SisusCo mentioned the nameof expression. It provides a string constant that matches the name of the symbol. So, when you rename that symbol in your code, it gets renamed everywhere you are using nameof. It's commonly used in the .NET system library for case like throwing an ArgumentNullException(nameof(theArgument)). If you rename the argument everything still works, and you can easily find all of the nameof(symbol) references the way you would normally search for that symbol.

 

You misunderstood, I was specifically referring to references to GameObject names, Tag names, paths, etc., some strings in the Unity editor, Visual Studio doesn't have string IntelliSence, Unity and VS can work better together. P.S: I remember PyCharm has string autocompletion.

 

None of those are 'magic strings'. And, honestly, none of those you should be using on the regular either, as they're all generally unreliable or limiting when it comes to design or architecture.

 

There are ways to get around having to repeat the same name/tag/path/layer etc. in multiple places as magic strings (hardcoded strings not assigned to any variable) or otherwise.

Code like this should be avoided:

Code (CSharp):

1. bool IsPlayer() => CompareTag("Player");

Instead, the string can be grouped with other similar strings in a dedicated class as a constant field:

Code (CSharp):

1. public static class Tags
2. {
3.     public const string Player = "Player";
4. }
5.  
6. bool IsPlayer() => CompareTag(Tags.Player);

It is even possible to auto-generate classes like this for tags, layers and such, ensuring that they are in-sync with the values configured in the editor.


Another option is to replace the string with an object that is drawn in the Inspector as a dropdown field, which doesn't allow selecting invalid options and contains validation logic:

Code (CSharp):

1. [SerializeField] Tag tag;
2.  
3. bool IsPlayer() => CompareTag(tag); // implicitly converted to string only when absolutely necessary

For example, instead of a raw path to an asset, an AssetReference to an addressable could be used instead in many cases.


And instead of finding a game object by its name, perhaps a component can be attached to it instead which uniquely identifies it. Or perhaps an enum can be used to specify all the different objects of a certain kind that can be found in the hierarchy.

Code (CSharp):

1. bool IsPlayer() => TryGetComponent(out Entity entity) && entity.Type == EntityType.Player;