Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify or add contructors for defaultExpiry = null #40

Open
jovball opened this issue Dec 27, 2023 · 1 comment
Open

Modify or add contructors for defaultExpiry = null #40

jovball opened this issue Dec 27, 2023 · 1 comment
Assignees

Comments

@jovball
Copy link

jovball commented Dec 27, 2023

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");
@xin9le
Copy link
Owner

xin9le commented Dec 29, 2023

@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 xin9le self-assigned this Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants