Joystick API design suggestion

[Gambit]

Hi all, I have been reading over the API for the joystick in SFML and it seems a little counter-intuitive. All methods in the sf::Joystick class are static and I dont understand why. If I may offer a suggestion, sf::Joystick should be an object constructed with an int (Being the device index), and all other methods act on that. For example:

sf::Joystick oJoystick(0);
oJoystick.hasAxis(sf::Joystick::X);
 

I understand that such a change in the API would cause a lot of problems with backwards compatibility but it better suits OOP and it looks better. Some people might be thinking that if this change were to go through, sf::Mouse and sf::Keyboard should also be objects except that as far as I know, SFML does not support multiple keyboards and mice, but the joystick API supports multiple joysticks so sf::Joystick being an object makes a bit more sense and in the long-run the code looks better and so does the API in my opinion.

[Ixrec]

I assume the reason is that you can't construct and destruct joysticks from your SFML code, just as you can't construct or destruct keyboards or mice.  The user has to plug in these physical devices, and all you can ever do is query the state of those devices, so the only real advantage of this would be allowing the user to omit a joystick number from most of the functions (and in my experience, I always pull those numbers out of events rather than store them myself, so that isn't actually a useful feature imo).  At a high level, it's generally best to design interfaces that accurately reflect what the class is actually doing (ie, querying a joystick's state, not constructing and managing joysticks itself).  This helps minimize fundamental, conceptual errors about what the class can do, especially for newbies.

That said, it's trivial to create your own wrapper class around sf::Joystick that simply stores a joystick number and uses it when calling all the real methods, so if you find that has some advantage over taking the joystick numbers from sf::Events and passing them around (which for me it doesn't), feel free to do that.

Edit: For some reason I felt like actually writing the wrapper to show how trivial it is:

class JoystickWrapper {
public:
    JoystickWrapper(unsigned int num): d_joystickNum(num) {}
    bool isConnected()                        { return sf::Joystick::isConnected(d_joystickNum); }
    unsigned int getButtonCount()             { return sf::Joystick::getButtonCount(d_joystickNum); }
    bool hasAxis(Axis axis)                   { return sf::Joystick::hasAxis(d_joystickNum, axis); }
    bool isButtonPressed(unsigned int button) { return sf::Joystick::isButtonPressed(d_joystickNum, button); }
    float getAxisPosition(Axis axis)          { return sf::Joystick::getAxisPosition(d_joystickNum, axis); }
private:
    unsigned int d_joystickNum;
}
 

[Gambit]

Aside from a nicer API and less needed code for the end user its true that there is virtually no advantage to the suggestion. While I'm not insisting that this should be considered, I personally dont think such a change would hurt, but that being said I dont use the joystick API myself but I was planning on writing a profile which inherits from the joystick class. Honestly, the idea of passing an index number to each joystick method seems very C-style and it could be easily made to use objects instead. There is really no difference between calling

sf::Joystick::getButtonCount(unsigned int joystick)

and calling

oJoystick.getButtonCount()

Nicer end user API, same underlying API.

Edit: In response to your edit, thats basically what I'm talking about, except it would actually be part of SFML. In any case, I'll leave it up to you to decide which API is nicer.

[Hiura]

Such change would definitely not take place in SFML 2.x since, as you mentioned, it breaks the API. But it can still be discussed for future versions like 3.0 and we welcome such discussion.   

Now, personally I like the current API. To start with, I slightly disagree with you when you say «passing an index number to each joystick method seems very C-style» [where I understand that C-style is bad]. In some situations free functions (what you call C-style) are actually not bad. And I think that's one of those situations.

Why? Well, even if such class would provide a isConnected() function, you would typically assume that when you got a joystick object it is connected. Hence, you'll call isButtonPressed(x) on it without checking if it's connected or not – you'll forget to do it maybe simply by mistake.

When you have a free function, you (or, at least, I) think differently about the parameters: I'm more careful about the value I give to the function and therefore would check if the joystick is connected more systematically.

But since it's mostly a matter of taste, feel free to disagree. 

[Gambit]

Good point. Like I said I'm not going to insist  that it be implemented right now or even in future versions, just that because it supports multiple joysticks and I'm not a huge fan of free functions in this case, I thought I would propose an API suggestion. The wrapper that Ixrec is virtually what I'm suggesting so in case, a wrapper would work just as well.

[Laurent]

that being said I dont use the joystick API myself

Using an API is the first thing you should do before suggesting a replacement for it. Things that may seem valid at first sight may end up being more verbose or less easy to use in real cases.

[Gambit]

Yea... I should probably do that heh.