[
https://issues.apache.org/jira/browse/HADOOP-13010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15243735#comment-15243735
]
Kai Zheng commented on HADOOP-13010:
------------------------------------
bq. The only state in AbstractRawErasureCoder.java is configuration state. I
don't see why we need this class. Everything in there could and should be a
utility function.
{{AbstractRawErasureCoder}} maintains {{conf}}, schema related info like
{{numDataUnits}}, and {{coderOptions}}. It provides public methods (9 ones) to
access these fields. All of these are essentials to a erasure coder and common
to both encoders and decoders. If we move the variables and methods to a
utility class, it wouldn't look better, and we have to duplicate the methods
across encoder and decoder.
bq. To continue the analogy with Java, InputStream and OutputStream don't share
a common base class.
Yes it's interesting. I just thought of an exact match for the current codes.
In JRE sasl framework, it has interfaces SaslClient and SaslSever, abstract
classes AbstractSaslImpl and GssKrb5Base, class GssKrb5Client extends
GssKrb5Base implements SaslClient, and class GssKrb5Server extends GssKrb5Base
implements SaslServer. I'm not sure we followed the style but I guess it could
be a common pattern for a bit of complex situation. I thought that's why when
it initially went in this way people understood the codes and I heard no other
voice then.
bq. It seems reasonable, but I don't see the need for AbstractRawErasureCoder.
Generally and often, I have to admit that I'm more a OOP guy and prefer to have
clear construct over concept and abstract, rather than mixed utilities. We can
see many example utilities in the codebase that are rather lengthy and messy,
which intends to break modularity. That's probably why I'm not feeling so well
to get rid of the level and replace it with utilities here. I agree with you
that sometimes composition is good to reuse some codes to avoid complex
inheritance relationships, but here we do have a {{coder}} concept and the
construct for it wouldn't be bad to have.
> 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
>
>
> 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)
