6

My question is pretty simple, but I didn't find a way to implement my code the way I want it to be. So I started wondering if the code I want to implement is not good. And if it is, what's the best way to do it.

Here it goes:

class InputManager  
{  
    SortedDictionary<ushort,Keys> inputList = new SortedDictionary<ushort,Keys>();  

    public void Add(ushort id, Keys key) {...}  
    public bool IsPressed(ushort id) {...}  
}  

class Main  
{  
    private enum RegisteredInput : ushort  
    {  
        Up,  
        Down,  
        Confirm  
    }  

    public Main()  
    {  
            InputManager manager = new InputManager();

            manager.Add(RegisteredInput.Up, Keys.Q);
            manager.Add(RegisteredInput.Down, Keys.A);
            manager.Add(RegisteredInput.Confirm, Keys.Enter);
    }

    void update()
    {
    if(manager.IsPressed(RegisteredInput.Up)) action();
    }
}

This code won't compile, giving errors of this kind:

The best overloaded method match for 'InputManager.Add(ushort, Keys)' has some invalid arguments
Argument '1': cannot convert from 'RegisteredInput' to 'ushort'

If I use a cast like in manager.Add((ushort)RegisteredInput.Up, Keys.Q); it will work. But because the cast must be explicit, I was wondering if it is not recomended code in C# like it is in C++ and if there is a better way of doing it (like using const ushort for every value, which I kinda don't like much).

The best answer I got so far was from this thread, but it sounds so much like a hack, I got worried.

Thanks!

Community
  • 1
  • 1
  • 5
    Why isn't the dictionary defined as `Dictionary`? Also, try to avoid retyping error messages and code into websites when you ask about things, the chance that you do something wrong we will get hung up on is there. Like... did the exception message actually name the method "Addd" with 3 d's? How can we trust you to actually copy the actual code used, and not retype something simplified that has other problems altogether? – Lasse V. Karlsen Feb 25 '10 at 20:52
  • Because I don't want repeated keys. About the error message, I didn't retype, just removed lot of irrelevant namespace from it. I also changed the real names of the methods, because they are not in english. – Ricardo Inacio Feb 25 '10 at 20:57
  • I don't understand what you mean by "repeated keys". Can you elaborate? – Lasse V. Karlsen Feb 25 '10 at 21:14
  • Hmm, seems like I didn't get your suggestion correctly at first. Sorry! I thought you were questioning whether to use Dictionary instead of SortedDictionary. Your suggestion is pretty much the same from Adam Robinson and leppie down there. Sorry again! – Ricardo Inacio Feb 25 '10 at 21:30

3 Answers3

7

Make InputManager a generic type. IE:

class InputManager<T>
{
   SortedDictionary<T,Keys> inputList = new SortedDictionary<T,Keys>();  

   public void add(T id, Keys key) {...}  
   public bool isPressed(T id) {...}    
}
leppie
  • 115,091
  • 17
  • 196
  • 297
7

Why not just define the dictionary using your enumeration? Is there a reason it needs to be an int?

public void add(RegisteredInput id, Keys key) {...}  

Also, as an aside, it's generally recommended that publicly-acessible members (methods, types, etc.) should be pascal cased (in other words, Add instead of add).

Adam Robinson
  • 182,639
  • 35
  • 285
  • 343
  • About the pascal cased, it is in the original. I'll correct it here too. Sorry! I don't want to use the enum as key directly to avoiding coupling. – Ricardo Inacio Feb 25 '10 at 21:08
5

The implicit cast is necessary for Enums I recommend this:

public static class RegisteredInput {
    public const ushort Up = 0;
    public const ushort Down = 1;
    public const ushort Confirm = 2;
}
Michael Pardo
  • 2,590
  • 3
  • 24
  • 33
  • Well then... beat my by a minute! Ditto, obviously! – Mark Carpenter Feb 25 '10 at 20:55
  • But do you actually think this is a better way of writing this code, c#-wise? I don't like the fact that I have to simulate the basic working of an enum that is giving values to enumerations automatically. – Ricardo Inacio Feb 25 '10 at 21:04
  • A lot of technical issues that I have run into in the past have me steering away from Enums recently. I use this practice for all my constants whether they be ints, strings, dates, etc. It's a little more work than an Enum, but not by much. – Michael Pardo Feb 25 '10 at 21:08
  • I, myself, also do try to avoid enums when they have to get outside of the class they are declared at (or the code block). I do consider `private enums ` pretty safe. – Ricardo Inacio Feb 25 '10 at 21:13
  • I agree. Although the underlying type is a ushort, when using enums you've abstracted away from the ushort, if just a little bit, and the compiler is complaining that you're using enums for a non-enum reason, thus the explicit cast. If you don't want the cast you either need to change the signature to support enums, switch to class variables or just overload Add() to support both enums and ushorts. – Chris Haas Feb 25 '10 at 21:18