accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Keith Turner (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ACCUMULO-2493) BinaryFormatter needs to be refactored
Date Mon, 07 Dec 2015 18:30:11 GMT

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

Keith Turner commented on ACCUMULO-2493:
----------------------------------------

It may be nice to pass in a Connector and table name also.  If passing these, then its more
than config its also environmental info.  So I tried changing the name of FormatterConfig
to FormatterEnv below (but I could also see leaving the name as FormatterConfig). Also the
DateFormatGenerator seems a lot like a Guava supplier (from the perspective of someone implementing
a formatter).  Also, maybe FormatterEnv should be an interface, with implementation package
private.

{code:java}  
  public interface FormatterEnv {
     String getTableName();
     Connector getConnector();
     Supplier<DateFormat> getDateFormatSupplier();
     public boolean willPrintTimestamps();
     public boolean willLimitShowLength();
     public int int getShowLengthToShow();
  }

  public interface Formatter extends Iterator<String> {
    void init(Iterable<Entry<Key,Value>> scanner, FormatterEnv env);
  }
{code}

The reason I am thinking of passing a {{Connector}} is that it would allow an implementation
to access custom table properties (added by ACCUMULO-2841).    {{Connector}} is already in
the public API and it would allow an implementation to do many other things like do lookups
in another table for formatting.

Also, I am not sure about the names {{willPrintTimestamps}} and {{willLimitShowLength}}. 
 In my mind the implementation can optionally honor these (for example the implementation
may never print timestamps or always print timestamps).  Not sure what naming scheme would
imply these are optional.   For example [FluoFormatter|https://github.com/fluo-io/fluo/blob/b16774ac9068a904e6da2741f5d953a6866e4cb9/modules/accumulo/src/main/java/io/fluo/accumulo/format/FluoFormatter.java]
 will always print the timestamp, regardless of the setting. 


> BinaryFormatter needs to be refactored
> --------------------------------------
>
>                 Key: ACCUMULO-2493
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-2493
>             Project: Accumulo
>          Issue Type: Bug
>          Components: client
>            Reporter: Mike Drob
>            Assignee: Matt Dailey
>              Labels: newbie
>             Fix For: 1.7.1, 1.8.0
>
>
> BinaryFormatter is currently used in a couple places in the shell, but the code is hard
to read and understand. There is a static getlength, which is actually a setter, and all the
instance calls end up going through unnecessary static methods.
> This combination makes it hard to reuse BinaryFormatter objects, or even use multiple,
since the static state is likely to conflict.



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

Mime
View raw message