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

Kai Zheng commented on HADOOP-13010:
------------------------------------

Thanks Colin.
bq.  It sounds like Encoder and Decoder will be stateless once they're created. 
Basically, they just reflect an algorithm and some data required to implement 
that algorithm. That is reasonable. We should document this in the JavaDoc for 
the classes.
Yeah, it's good to document this stateless property in JavaDoc. Note by the way 
it doesn't mean these encoder/decoder are to support concurrency though it's 
possible. I would leave this for future consideration. 

bq. The problem with calling this class DecoderState is that this name suggests 
that it is the state of the coder, rather than state manipulated by the coder. 
Perhaps calling this DecoderData or DecoderStream is more appropriate. Having 
this new class will also avoid the need to manually pass around so many arrays 
and indices.
Ah yes the names (EncoderState/DecoderState) are bad, actually I meant them to 
be *EncodingState/DecodingState*.

bq. These methods and fields are configuration methods and configuration 
fields. They belong in a class named something like 
"ErasureEncodingConfiguration" or something like that. ...Why is having the 
configuration in a separate object better than having the configuration in a 
base class?
Ok, I'm probably convinced by you. Thanks for the lots of insights. I got rid 
of the base class anyway, and introduced *ErasureCoderConf* for the variables 
and methods in it. As you might check the updated patch, there are some 
duplicate of small shortcuts between the encoder base class and decoder base 
class as they now lack a common base. I suppose it's acceptable.

> Refactor raw erasure coders
> ---------------------------
>
>                 Key: HADOOP-13010
>                 URL: https://issues.apache.org/jira/browse/HADOOP-13010
>             Project: Hadoop Common
>          Issue Type: Sub-task
>            Reporter: Kai Zheng
>            Assignee: Kai Zheng
>             Fix For: 3.0.0
>
>         Attachments: HADOOP-13010-v1.patch, HADOOP-13010-v2.patch, 
> HADOOP-13010-v3.patch
>
>
> This will refactor raw erasure coders according to some comments received so 
> far.
> * As discussed in HADOOP-11540 and suggested by [~cmccabe], better not to 
> rely class inheritance to reuse the codes, instead they can be moved to some 
> utility.
> * Suggested by [~jingzhao] somewhere quite some time ago, better to have a 
> state holder to keep some checking results for later reuse during an 
> encode/decode call.
> This would not get rid of some inheritance levels as doing so isn't clear yet 
> for the moment and also incurs big impact. I do wish the end result by this 
> refactoring will make all the levels more clear and easier to follow.



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

Reply via email to