7digital / SevenDigital.Api.Wrapper

A fluent c# wrapper for the 7digital API
MIT License
16 stars 29 forks source link

WithIntParameter to convert params from int to string #178

Closed AnthonySteele closed 10 years ago

AnthonySteele commented 10 years ago

We convert params from int to string all over the place And from decimal in a few places and we don't always do it the same way So centralise it and it the long-winded right way just once (in ParameterExtensions.cs) and use that internally and expose for external use. New extension methods are WithIntParameter(name, value) and WithDecimalParameter(name, value)

gregsochanik commented 10 years ago

How about going with an overload on WithParameter<T> rather than adding WithIntParameter and WithDecimalParameter

i.e.

public static class ParameterExtensions
{
    public static IFluentApi<T> WithParameter<T>(this IFluentApi<T> api, string name, int value)
    {
        api.WithParameter(name, value.ToString(CultureInfo.InvariantCulture));
        return api;
    }

    public static IFluentApi<T> WithParameter<T>(this IFluentApi<T> api, string name, decimal value)
    {
        api.WithParameter(name, value.ToString(CultureInfo.InvariantCulture));
        return api;
    }
}

?

AnthonySteele commented 10 years ago

Good idea... though I get this.

public static IFluentApi<T> WithParameter<T, TV>(this IFluentApi<T> api, string name, TV value)
{
    api.WithParameter(name, value.ToString(CultureInfo.InvariantCulture));
    return api;
}

But that doesn't compile - the ToString(CultureInfo) method isn't present. Do we use any other types at all frequently besides string, int, decimal?

... but public static IFluentApi<T> WithParameter<T>(this IFluentApi<T> api, string name, int value) seems to work just fine.

AnthonySteele commented 10 years ago

I have updated the PR: Rename WithXXXParameter() to WithParameter(). This works fine.