7
\$\begingroup\$

I'm creating a game in C#, and I've created a ScreenManager so I can call

ScreenManager.MoveToScreen(typeof(ScreenClassHere))

to move to other game states. However, I feel that my method of creating an instance of the class is... not very optimal. See for yourself.

    public static void MoveToScreen(Type s)
    {
        if (currentscreen != null) currentscreen.Remove();

        currentscreen = (Screen)s.GetConstructor(System.Type.EmptyTypes).Invoke(null);
        currentscreen.Init();
    }

Does anyone know if there's a better way to create an instance of a certain class from a Type?

\$\endgroup\$
19
\$\begingroup\$

You could use generics

public static void MoveToScreen<T>() where T : Screen, new()
{
    if (currentscreen != null) currentscreen.Remove();

    currentscreen = new T();
    currentscreen.Init();
}

In this case, you'd have to call the method as

ScreenManager.MoveToScreen<ScreenClassHere>();
\$\endgroup\$
  • \$\begingroup\$ Let's get those proper casts in there. :) \$\endgroup\$ – ChaosPandion Mar 1 '11 at 20:05
  • \$\begingroup\$ Although this seems like the better way to do it, I'm getting errors about the compiler not being able to convert T to Screen... Error 1 Cannot convert type 'T' to 'SFMLGame.Screens.Screen' \$\endgroup\$ – Dlaor Mar 1 '11 at 20:06
  • 1
    \$\begingroup\$ Since currentscreen seems to have the type Screen this should be where T : Screen, new(). \$\endgroup\$ – sepp2k Mar 1 '11 at 20:07
  • \$\begingroup\$ @sepp2k - Exactly I was gonna suggest that over an awkward cast currentscreen = (Screen)(object)new T();. \$\endgroup\$ – ChaosPandion Mar 1 '11 at 20:09
5
\$\begingroup\$

I don't like the naming of a Remove method, it suggests that you remove something from the currentScreen but really it's the method that is called when the currentScreen itself is being removed, if I got it right.

Also, other people suggesting generics are correct, you should use it. But if you once again find yourself in need of creating an instance knowing its type, don't go GetConstructors way. Use Activator.CreateInstance for that.

\$\endgroup\$
4
\$\begingroup\$

Nitpicky, but I would change if (currentscreen != null) currentscreen.Remove(); to

if (currentscreen != null) {
   currentscreen.Remove();
}

I second pdr on using generics.

\$\endgroup\$
  • \$\begingroup\$ I don't like the unnecessary braces but I don't like it as a one-liner either, I would make it a two-liner with proper indentation. Just an opinion \$\endgroup\$ – Dyppl Mar 2 '11 at 5:02
  • \$\begingroup\$ Nothing wrong with doing that either. I started always using braces because it's clearer to modify. Personal preference :) \$\endgroup\$ – Michael K Mar 2 '11 at 14:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.