StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.85k stars 1.5k forks source link

Accept objects in constructor of RedisValue #2634

Closed SerhiyBalan closed 5 months ago

SerhiyBalan commented 5 months ago

Is it possible to add a public constructor for the RedisValue that would accept object value and automatically convert it to best possible format? RedisValue has lot of internal conversion tools, and they are not accessible

Right now I'm using the Facade pattern with methods like

   public async Task<T?> GetAsync<T>(string key)
    {
        var result = await _database.StringGetAsync(key);
        return result.ToObject<T>();
    }

    public Task SetAsync<T>(string key, T value, TimeSpan? expiry = null)
        => _database.StringSetAsync(key, value.ToRedisValue(), expiry);

Similar for other data types like Hash

In both cases I had to use extensions to convert data:

    internal static RedisValue ToRedisValue<T>(this T value)
    {
        var sourceType = typeof(T);
        if (sourceType.IsValueType || sourceType == typeof(string))
        {
            // Store all simple types as string
            return value?.ToString();
        }

        if (value is byte[] bytes)
        {
            // Redis supports byte arrays natively
            // Serialized version would be much larger
            return bytes;
        }

        // All other data types should be serialized as string
        return value.SerializeObject();
    }

Another option is to write code like

        if (value is int intValue)
        {
            return intValue;
        }

        if (value is double doubleValue)
        {
            return doubleValue;
        }

There are would be lot of ifs, because StackExchange.Redis has lot of internal convertors :(

The ToObject has similar code but from RedisValue to T.


So I would be interested in come public RedisValue(object value) that would try to detect best matching target format, or serialize the string if not possible.

And some method like ConvertTo

        var result = await _database.StringGetAsync(key);
        return result.ConvertTo<T>();

Thank you

mgravell commented 5 months ago

There's two big problems with that, IMO:

  1. it causes and encourages boxing allocation overhead, which ... isn't necessary here
  2. it provides no feedback about types that won't work, and almost everything can be cast to object

So; at the worst case it could cause frustrating scenarios where problems that today are clear at build-time are deferred to runtime, and at best: it works but is unnecessarily expensive

I think I'd need a bit more "pro" to outweigh these "cons", especially since this could already be achieved in a local utility method if you really want that usage yourself