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

Appy commented on HBASE-19675:
------------------------------

bq. I think it's best to always assume that the Collection being returned from 
a method should not be directly manipulated, only iterated
In that case, it's probably better to choose Iterable<T> as return type.

My earlier point was about params though. Taking in params which better 
communicate they way in which they are used, if possible. For instance, taking 
in an Object (say List, Collection, etc) as Iterable/Stream (read-only 
datastructures) makes it clear that a function is not modifying it.

Anyways, let me commit your patch.

> Miscellaneous HStore Class Improvements
> ---------------------------------------
>
>                 Key: HBASE-19675
>                 URL: https://issues.apache.org/jira/browse/HBASE-19675
>             Project: HBase
>          Issue Type: Improvement
>          Components: hbase
>    Affects Versions: 3.0.0
>            Reporter: BELUGA BEHR
>            Assignee: BELUGA BEHR
>            Priority: Minor
>         Attachments: HBASE-19675.1.patch, HBASE-19675.2.patch
>
>
> * Remove logging code guards in favor of slf4j parameters
> * Use {{CollectionsUtils.isEmpty()}} consistently
> * Small check-style fixes
> * Remove flow control logic from {{trace}} statement {code}          if 
> (LOG.isTraceEnabled()) {
>             LOG.trace("No compacted files to archive");
>             return;
>           }{code}
> * Replace two calls to the same getter to ensure that the value doesn't 
> change between calls {code}      if (getCompactedFiles() != null) {
>         for (HStoreFile file : getCompactedFiles()) {
>           name2File.put(file.getFileInfo().getActiveFileName(), file);
>         }
>       }{code}
> * Make 'inputFiles' a Set for fast calls to {{contains}} method instead 
> {code}    //some of the input files might already be deleted
>     List<HStoreFile> inputStoreFiles = new 
> ArrayList<>(compactionInputs.size());
>     for (HStoreFile sf : this.getStorefiles()) {
>       if (inputFiles.contains(sf.getPath().getName())) {
>         inputStoreFiles.add(sf);
>       }
>     }{code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to