nonbinaryprogrammer commented on a change in pull request #6599:
URL: https://github.com/apache/geode/pull/6599#discussion_r651310648
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/server/SlowlogExecutor.java
##########
@@ -24,18 +29,16 @@
public class SlowlogExecutor extends AbstractExecutor {
@Override
- public RedisResponse executeCommand(Command command,
- ExecutionHandlerContext context) {
- String subCommand = command.getStringKey().toLowerCase();
- switch (subCommand) {
- case "get":
- return RedisResponse.emptyArray();
- case "len":
- return RedisResponse.integer(0);
- case "reset":
- return RedisResponse.ok();
- default:
- return null; // Should never happen
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
Review comment:
same comment here as above
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/server/InfoExecutor.java
##########
@@ -50,44 +61,29 @@ private boolean containsSectionParameter(List<byte[]>
commands) {
return commands.size() == 2;
}
- private String getSpecifiedSection(ExecutionHandlerContext context,
- List<byte[]> commands) {
- String result;
- String section = Coder.bytesToString(commands.get(1)).toLowerCase();
- switch (section) {
- case "server":
- result = getServerSection(context);
- break;
- case "cluster":
- result = getClusterSection();
- break;
- case "persistence":
- result = getPersistenceSection();
- break;
- case "replication":
- result = getReplicationSection();
- break;
- case "stats":
- result = getStatsSection(context);
- break;
- case "clients":
- result = getClientsSection(context);
- break;
- case "memory":
- result = getMemorySection(context);
- break;
- case "keyspace":
- result = getKeyspaceSection(context);
- break;
- case "default":
- case "all":
- result = getAllSections(context);
- break;
- default:
- result = "";
- break;
+ private String getSpecifiedSection(ExecutionHandlerContext context,
List<byte[]> commands) {
Review comment:
this seems to be less efficient because it results in lots of calls to
changing the case of the letters, instead of just the one conversion on the left
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/ZAddExecutor.java
##########
@@ -69,39 +75,28 @@ private int findAndValidateZAddOptions(Command command,
Iterator<byte[]> command
int optionsFoundCount = 0;
while (commandIterator.hasNext() && !scoreFound) {
- String subCommandString =
Coder.bytesToString(commandIterator.next()).toLowerCase();
- switch (subCommandString) {
- case "nx":
- executorState.nxFound = true;
- optionsFoundCount++;
- break;
- case "xx":
- executorState.xxFound = true;
- optionsFoundCount++;
- break;
- case "ch":
- executorState.chFound = true;
- optionsFoundCount++;
- break;
- case "inf":
- case "+inf":
- case "-inf":
- case "infinity":
- case "+infinity":
- case "-infinity":
- scoreFound = true;
- break;
- case "incr":
- executorState.incrFound = true;
- optionsFoundCount++;
- break;
- default:
- try {
- Double.valueOf(subCommandString);
- } catch (NumberFormatException nfe) {
- executorState.exceptionMessage = ERROR_NOT_A_VALID_FLOAT;
- }
- scoreFound = true;
+ byte[] subCommand = commandIterator.next();
Review comment:
also concerned about all the changing case here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
