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

Lars George commented on HBASE-13270:
-------------------------------------

Since the I am not sure what the original intent was, maybe [~jesse_yates] 
could chime in?

>From the usage which does this

{code}
        } else if (roe.hasResult()) {
          responseValue = ProtobufUtil.toResult(roe.getResult(), cells);
          // add the load stats, if we got any
          if (roe.hasLoadStats()) {
            ((Result) responseValue).addResults(roe.getLoadStats());
          }
{code}

it looks like a one off call, i.e. straight assignment as the code does 
internally already. In this case I am +1 for naming it {{setStats()}} instead 
as you suggest.

In fact, I am a hater for these short names anyways (all IDEs with 
autocompletion take this pain away these days) and would rather suggest we use 
{{setStatistics()/getStatistics()}} - and since this is arbitrary, even better 
{{setRegionLoadStatistics()/getRegionLoadStatistics()}}. This makes it much 
more user friendly without having to resort to JavaDoc and other measures. 

> Setter for Result#getStats is #addResults; confusing!
> -----------------------------------------------------
>
>                 Key: HBASE-13270
>                 URL: https://issues.apache.org/jira/browse/HBASE-13270
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: stack
>              Labels: beginner
>         Attachments: HBASE-13270.patch
>
>
> Below is our [~larsgeorge] on a finding he made reviewing our API:
> "Result class having getStats() and addResults(Stats) makes little sense..."
> "...the naming is just weird. You have a getStats() getter and an 
> addResults(Stats) setter???"
> "...Especially in the Result class and addResult() is plain misleading..."
> This issue is about deprecating addResults and replacing it with addStats in 
> its place.
> The getStats/addResult is recent. It came in with:
> {code}
> commit a411227b0ebf78b4ee8ae7179e162b54734e77de
> Author: Jesse Yates <jesse.k.ya...@gmail.com>
> Date:   Tue Oct 28 16:14:16 2014 -0700
>     HBASE-5162 Basic client pushback mechanism
> ...
> {code}
> RegionLoadStats don't belong in Result if you ask me but better in the 
> enveloping on invocations... but that is another issue.



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

Reply via email to