I'm personally a big fan of using enums to constrain my public APIs. The more constraints, the easier it is to test all permutations of inputs. If that's a goal you're interested in achieving, I do have some suggestions. (And if not, feel free to skip to the end of this response!)
I'll use the gethistory method as an example. Even though it is documented that callers should use the History enum, there is nothing in this method that enforces that constraint. The test for this method shows that the History enum is expected to be used on the RequestBuilder return type. This is a flexible design, but perhaps it is too flexible?
The following example compiles, but will probably have problems at runtime because it sends invalid query parameters to the server:
initialize_client()
.gethistory("TRP")
.params(&[("foo", String::from("bar"))])
.execute();
We can constrain this API by keeping the RequestBuider internal to it, and accept a slice of History values:
pub fn gethistory<T>(&self, symbol: &str, params: &[History]) -> T
where
RequestBuilder: Execute<T>,
{
let mut builder = self.client
.get(format!("{}marketdata/{}/pricehistory", APIWWW, symbol));
for param in params.iter().cloned() {
let (key, value): (&str, String) = param.into();
builder = builder.param(key, value);
}
builder.execute()
}
I had to derive Clone for History because we're not allowed to move out of &[T]. One workaround is to implement Into<(&'static str, String)> for &History<'a>. Another option would be providing a method like fn as_kv(&self) -> (&str, String) to avoid unnecessary cloning. It looks much better, too:
for param in params {
let (key, value) = param.as_kv();
builder = builder.param(key, value);
}
With that, the test can be rewritten:
#[test]
fn able_to_retrieve_history() {
let resptxt: String = initialize_client()
.gethistory("TRP", &[
History::Period(1),
History::PeriodType("month"),
History::Frequency(1),
History::FrequencyType("daily"),
]);
println!("{:?}", resptxt);
assert_eq!(resptxt.contains("\"candles\""), true);
}
And now the caller can't provide invalid inputs.
Caveat: Arguably the string and integer values are still unconstrained! It is possible to construct invalid values like History::PeriodType("foo"). These of course can be further constrained with additional enums and structs. Use a PeriodType enum in place of free-form strings. Make the integer types "private" by wrapping them in a tuple struct and providing a static constructor method that validates the numeric value. If you choose to.
Yep, those implementations for Execute<T> are fine! You can see how in the rewrite of gethistory() above, I constrained the return type be any T for which RequestBuilder implements Execute<T>. Nothing else had to change on those implementations, and this function will be able to output either String or serde_json::Value!