CloudStructures icon indicating copy to clipboard operation
CloudStructures copied to clipboard

Modify or add contructors for defaultExpiry = null

Open jovball opened this issue 2 years ago • 1 comments

First of all, thanks for upgrading this library to .NET Core. It's a great tool and I'm coming back to it after several years away from using Redis.

The various data structs all require a TimeSpan? variable for the constructor. This makes sense when doing the various Add/Set/Remove type operations when you are changing the data. It doesn't make much sense to me for operation that are only retrieving the data.

Here is a code example to demonstrate.

// 1. Create redis structure
var key = "test-key";
var defaultExpiry = TimeSpan.FromDays(1);
var redis = new RedisString<Person>(RedisServer.Connection, key, defaultExpiry);

// 2. Call command
var neuecc = new Person
{
    Name = "neuecc",
     Age = 35 
};

await redis.SetAsync(neuecc);

// in a different process, get the value
// what purpose does the defaultExpiry parameter serve here?
var redis1 = new RedisString<Person>(RedisServer.Connection, key, defaultExpiry);

var result = await redis1.GetAsync();
Console.WriteLine(result.Value.Name); // neuecc

I realized that I can supply null for that parameter but it would be nicer to have a default of null for the constructor since the TimeSpan is already nullable. This would be an example for the RedisList struct constructor

    public RedisList(RedisConnection connection, RedisKey key, TimeSpan? defaultExpiry = null)
    {
        this.Connection = connection;
        this.Key = key;
        this.DefaultExpiry = defaultExpiry;
    }

With that change, this pattern could be used

// changing data
var redisA = new RedisList2(RedisServer.Connection, "test-key", TimeSpan.FromDays(1));

// fetching data
var redisB = new RedisList2(RedisServer.Connection, "test-key");

jovball avatar Dec 27 '23 17:12 jovball

@jovball

Thank you for your suggestion, and we are grateful for your use of our service ;)

Regarding your proposal to pass a null argument to the constructor TimeSpan?, it's important to note that we intentionally did not set this as an argument. Setting the expiration to null implies that the key's lifespan is indefinite, which is not ideal when using caches like Redis. In other words, we don't want our library to suggest an indefinite lifespan. That's why we have designed it this way, so that if users want to set the lifespan to indefinite, they should do it explicitly.

We hope this clears up any confusion.

xin9le avatar Dec 29 '23 09:12 xin9le