Remove Redis::Client class

[kostya]

i suggest such diff to fix all of this, and remove not_nil! at all (but with client it seems it was simpler):

diff --git a/src/redis.cr b/src/redis.cr
index 3a35578..65e06a9 100644
--- a/src/redis.cr
+++ b/src/redis.cr
@@ -120,18 +120,20 @@ class Redis

   # :nodoc:
   private def connection : Redis::Connection
-    connect unless @connection
-    @connection.not_nil!
+    @connection || connect[0]
   end

   # Connects to Redis.

   # :nodoc:
   private def connect
-    @connection = Connection.new(@host, @port, @unixsocket, @ssl_context, @dns_timeout, @connect_timeout, @command_timeout)
-    @strategy = Redis::Strategy::SingleStatement.new(@connection.not_nil!)
-    strategy.command(["AUTH", @password]) if @password
-    strategy.command(["SELECT", @database.to_s]) if @database
+    @connection = _connection = Connection.new(@host, @port, @unixsocket, @ssl_context, @dns_timeout, @connect_timeout, @command_timeout)
+    @strategy = _strategy = Redis::Strategy::SingleStatement.new(_connection)
+
+    _strategy.command(["AUTH", @password]) if @password
+    _strategy.command(["SELECT", @database.to_s]) if @database
+
+    {_connection, _strategy}
   end

   # :nodoc:
@@ -187,7 +189,7 @@ class Redis

   # :nodoc:
   private def strategy : Redis::Strategy::Base
-    @strategy.not_nil!
+    @strategy || connect[1]
   end

   # Returns the server URL for this client.
@@ -237,8 +239,8 @@ class Redis
     close
     raise ex
   ensure
-    if @connection
-      @strategy = Redis::Strategy::SingleStatement.new(@connection.not_nil!)
+    if _connection = @connection
+      @strategy = Redis::Strategy::SingleStatement.new(_connection)
     end
   end

@@ -274,8 +276,8 @@ class Redis
     close
     raise ex
   ensure
-    if @connection
-      @strategy = Redis::Strategy::SingleStatement.new(@connection.not_nil!)
+    if _connection = @connection
+      @strategy = Redis::Strategy::SingleStatement.new(_connection)
     end
   end

diff --git a/src/redis/commands.cr b/src/redis/commands.cr
index b9e2fd0..2ad0eac 100644
--- a/src/redis/commands.cr
+++ b/src/redis/commands.cr
@@ -1608,7 +1608,7 @@ class Redis
     end

     private def already_in_subscription_loop?
-      strategy.is_a? Redis::Strategy::SubscriptionLoop
+      @strategy.is_a? Redis::Strategy::SubscriptionLoop
     end

     # Unsubscribes the client from the given channels, or from all of them if none is given.

[stefanwille]

@kostya thank you. I don't see that this is necessary for the reason stated above.

[kostya]

i think this is possible to remove nilable values fully. just initialize in initialize and reinitialize in reconnect. to avoid @connection.not_nil! which is pretty not safe.

[stefanwille]

Wanting the option to make the connection nilable was the reason for you to introduce the @client attribute and the Redis::Client class, if I am not mistaken? That role has @connection now.

What is your idea?

[kostya]

you use not_nil! which is pretty not safe, i suggest fix in https://github.com/stefanwille/crystal-redis/pull/63#issuecomment-399371458