anoopsjohn commented on a change in pull request #3177:
URL: https://github.com/apache/hbase/pull/3177#discussion_r616309936



##########
File path: 
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RowCounter.java
##########
@@ -318,12 +324,10 @@ protected void processOptions(CommandLine cmd) throws 
IllegalArgumentException{
     if(cmd.getOptionValue(OPT_RANGE)!=null) {
       this.rowRangeList = 
parseRowRangeParameter(cmd.getOptionValue(OPT_RANGE));
     }
-    this.endTime = cmd.getOptionValue(OPT_END_TIME) == null ? 
HConstants.LATEST_TIMESTAMP :
-        Long.parseLong(cmd.getOptionValue(OPT_END_TIME));
-    this.expectedCount = cmd.getOptionValue(OPT_EXPECTED_COUNT) == null ? 
Long.MIN_VALUE :
-        Long.parseLong(cmd.getOptionValue(OPT_EXPECTED_COUNT));
-    this.startTime = cmd.getOptionValue(OPT_START_TIME) == null ? 0 :
-        Long.parseLong(cmd.getOptionValue(OPT_START_TIME));
+    this.startTime = getOptionAsLong(cmd, OPT_START_TIME, 0L);
+    this.endTime = getOptionAsLong(cmd, OPT_END_TIME, 
HConstants.LATEST_TIMESTAMP);
+    this.expectedCount = getOptionAsLong(cmd, OPT_EXPECTED_COUNT, 
Long.MIN_VALUE);
+    this.cacheBlocks = cmd.hasOption(OPT_CACHE_BLOCKS) ? true : false;

Review comment:
       What u mean?
   The state var is a variable and assign it with o/p of the check is enough.
   We dont need like if hasOption() the set to true or else set to false.
   Its a minor thing though.
   So instead of having
    this.cacheBlocks = cmd.hasOption(OPT_CACHE_BLOCKS) ? true : false;
   Below is enough
   this.cacheBlocks = cmd.hasOption(OPT_CACHE_BLOCKS) ;
   




-- 
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]


Reply via email to