f2e-spec
f2e-spec copied to clipboard
alibaba
Song/Player database ID handling needs refactoring
Do you want to request a feature or report a bug?
Not a bug per-se
What did you do?
Stumbled into this while elaborating a fix for #556; in Players, Songs and PlayerItem class there are some really fucked up members defined as int, like int empty()...
Then there's the way player IDs is handled; it seems it's only int so it's possible to pass a -1 for "not found" but even that makes little sense considering the current logic has it so the first valid ID is 1; so even keeping that logic, we could use size_t and 0 for invalid.
Also semi-related, we have a bunch of plain size_t here and there; I think we should homogeneize them all to std::size_t.
What did you expect to see?
bools should be bools, ids should be size_t.
Can you tell me the difference between size_t and int? passing -1 is a very common practise which this probably derives from
size_t is unsigned and 64 bit (in practice, not by standard of course), int is 32 bits and signed. This mismatch causes compiler warnings about unsigned vs. signed comparisons, and could lead to very real security issues if one could somehow overflow the comparison. There's also signed ssize_t, very rarely used anywhere.
It is a good idea to use size_t for anything that is a count, and throw an exception instead of returning special values like -1 (in C++, unlike in C).
I think @Lord-Kamina is right, bools should also be bools.
Ahh thanks for clarifing :)
With this explanation i think it should be done :)
-1 is used to initialize the value. That bit of logic could even be maintained (for initialization at least) frankly because 0 is not even being used either but the other things are ugly.
I would like to offer a pull request, but how can I test the Players/PlayerItem stuff? If I start performous it does not show a player screen.
Ok, now I found the player screen. I miss a screen to manage (create, remove, configure) players.