[ 
https://issues.apache.org/jira/browse/IGNITE-3066?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15664153#comment-15664153
 ] 

Alexey Kuznetsov commented on IGNITE-3066:
------------------------------------------

Roman, take a look on following comments:

1) I think isNx() and isXX should be refactore like this:
{code}
    private boolean isNx(List<String> params) {
        if (params.size() >= 3)
            return "nx".equalsIgnoreCase(params.get(0)) || 
"nx".equalsIgnoreCase(params.get(2));
        
        return "nx".equalsIgnoreCase(params.get(0));
    }

    private boolean isXx(List<String> params) {
        if (params.size() >= 3)
            return "xx".equalsIgnoreCase(params.get(0)) || 
"xx".equalsIgnoreCase(params.get(2));
        
        return "xx".equalsIgnoreCase(params.get(0));
    }
{code}

2) GridRedisIncrDecrCommandHandler#makeResponse
  Remove not needed {{else}} before {{return}}

3) Public constructor of abstract class {{GridRedisThruRestCommandHandler}} 
would be better declared as protected.

4) In {{GridRedisDelCommandHandler#asRestRequest}} while-loop could be replaced 
with for-each:
{code}
  Iterator<String> mgetIt = keys.iterator();
  while (mgetIt.hasNext())
      mget.put(mgetIt.next(), null);

  vs

  for (String key : keys)
     mget.put(key, null);
{code}
Same for {{GridRedisExistsCommandHandler}} and {{GridRedisMGetCommandHandler}}

5) How about to give more informative comments in  {{GridRedisCommand}} for 
commands?
For example I have no idea what is {{MGET}}?  Also it will be cool to give more 
comments for handlers.
for example {{GridRedisMGetCommandHandler}} Content could be taken from 
http://redis.io/commands/mget

6) May be rename {{GridRedisThruRestCommandHandler}} to 
{{GridRedisRestCommandHandler}} ?

7) {{GridRedisMessage}} is {{Serializable}}, but field {{private ByteBuffer 
response}} is not serializable?

8) Where {{IgniteNamedInstance#redisExecSvc}} is instantiated?

9) {{GridRedisProtocolParser#OkString}} is named not by Java method name 
convention (from upper case).

> Set of Redis commands that can be easily implemented via existing REST 
> commands
> -------------------------------------------------------------------------------
>
>                 Key: IGNITE-3066
>                 URL: https://issues.apache.org/jira/browse/IGNITE-3066
>             Project: Ignite
>          Issue Type: Sub-task
>            Reporter: Roman Shtykh
>            Assignee: Roman Shtykh
>
> As the 1st iteration of IGNITE-2788 the following commands can be implemented 
> via existing REST commands,
> GET
> MGET
> SET
> MSET
> INCR
> DECR
> INCRBY
> DECRBY
> APPEND
> STRLEN
> GETSET
> SETRANGE
> GETRANGE
> DEL
> EXISTS
> DBSIZE
> http://redis.io/commands



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to