Functions that return key/value data pairs should not return them as single strings #7
Description
Is your feature request related to a problem? Please describe.
Much of the data requested from the Toggl API exists in multiples of key/value pairs. In particular is the workspaces list and projects list. Currently, the library returns this data as a string and leaves it for the user to process. This isn't necessarily terrible, but formatting the data types to indicate the way the data should be worked with is valuable.
Describe the solution you'd like
Ideally, we'd like to return a std::arr type from these hypothetical getWorkspaces() and getProjects(). However, as the Arduino library doesn't support the std::arr, we need to implement a custom solution.
Structs are our friend in this scenario. A struct of the following form could capture both workspaces and projects:
struct KVPair {
String name;
int id;
};
There are a few problems with this though.
- We need to return an array of structs, and in this form we can't get the number of returned workspaces/projects
- An HTTP error can't intuitively gleamed from an empty array
There are 2 approaches we could use which will address both raised issues
- Get a pointer to an integer in the function call. The function updates this value with the amount of workspace/project entries in successful, or returns a NULL in the array of structs and sets this value to be the HTTP code upon failure.
- Wrap the array of workspace/project structs in a struct which contains a return integer value, something along the lines of
struct KVPairReturn {
int returnCode;
KVPair* dataPairs;
}
and return it as follows (in the case of a successful request)
KVPair* projects = new KVPair[arr.size()];
KVReturnPair returnData {
arr.size(),
projects
};
In case of a unsuccessful request, the projects array can be set to NULL and the returnCode value set to the HTTP return code.
In both cases the user can check if the data is valid by first checking if the returned array is NULL, and then if so check the HTTP code, if not use the updated integer value to know how many workspaces or projects they are working with. The data also represents the tie between the id and the name
Describe alternatives you've considered
Memory on an embedded system is scary. Giving strings definitely is easier to ensure things are safe, but I don't feel it's a good approach in this case as the data returned is non-intuitive and doesn't necessarily cater for more general use cases
I've tested the option KVPairReturn option, and it works rather nicely. It is also one less pointer than option 1, though we'd still need to make it clear to users to free up the KVPair array memory once they're done.
Additional context
Writing data structures to indicate the way the data is presented (and hence expected) is good practice. Returning strings is confusing to library users.