New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change encode method's arguement to OutputStream #2
Comments
and do casting to BerByteArrayOutputStream for issue beanit#2
the only problem I have with your idea, is that I leads to the false believe that the encode function could be used with any output stream. But actually the data is written in reverse order to the output stream and the BerByteArrayOutputStream takes special care of that fact by writing to a byte array starting at its end. |
Yes agree, there alreays stands the same point for decode and input stream. This PR is not a big deal but nice to have IMHO. |
Any more comments? |
Btw, in apache camel project, i implemented asn1 component using this library. If you wanna check out, please go ahead. This context of such proposal was to extend camel component without any direct dependency on jasn1. Please consider the change, let me know.. |
Ok it has the disadvantage that it makes you think you could use any outputstream but I guess I can live with that since it has the advantage that one can more easily use a different outputstream implementation. |
No no no, i think you make a wrong assumption what it makes me think. I know and am aware that the output stream should BerOutputStream. And what i want to have and suggest why not we could not have Outputstream in the method signature which would help me drop of static import in camel-asn1 component and maybe integrate another asn1 libabries easily. The reason of that the input stream again should be a proper input stream and that would not hurt, i suppose. I will include what you suggest in this PR. It is fair and rescues us from those ugly castings. You are right |
I did understand you! I was just saying that if we change to OutputStream in the method signature, that besides the advantage you mentioned, it has the second advantage that it would be theoretically feasible to implement another BerOutputStream that behaves somewhat different and use that one. |
when do you plan to make the changes to the PR? |
I have been traveling since 6th of october till 18th october. That week or over the same weekend, i will be able to look into it. |
@sfeuerhahn please see ref |
one more question. When do you think the new release may come out? I want to update the component as well. Could you please let me know? thanks |
Instead of having method signature as below;
public int encode(BerByteArrayOutputStream os) throws IOException
I would suggest having the signature like
public int encode(OutputStream os) throws IOException
This style would be more abstract and for example i would not need such import dependency in my camel-asn1 compiler where this would enable for third party developer's like me to have more loosly-coupled dependency in component library because i would not have such import
See;
https://github.com/onders86/camel/blob/CAMEL-ASN1/components/camel-asn1/src/main/java/org/apache/camel/dataformat/asn1/ASN1DataFormat.java#L35
Thanks
The text was updated successfully, but these errors were encountered: