hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jesse Yates (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-13270) Setter for Result#getStats is #addResults; confusing!
Date Fri, 20 Mar 2015 08:43:39 GMT

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

Jesse Yates commented on HBASE-13270:

I think the original naming came from the fact that the method calling addResults is managing
multiple Results, or maybe its adding the result load as a result of the last requests....neither
is necessarily a good reason, but that's all I can think of :(

[~larsgeorge], generally I agree with your style of naming as well; setStatistics()/getStatistics()
would certainly be better in this case, but this was to be the foundation of a more widespread
statistics mechanism, so setRegionLoadStatistics() didn't make as much sense in the longer
view. Not sure why I got lazy this time.

Sorry about this fellas - thanks for the cleanup.

+1 on the patch.

bq. RegionLoadStats don't belong in Result
There really wasn't a better place to put it, well, without completely re-architecting the
client (again). It was expedient, got the data where it needed to go - back into the client's
rpc caller. Happy to talk about how we can do it better :)

> 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.yates@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

View raw message