> On Feb. 2, 2015, 9:41 p.m., Rafael Schloming wrote: > > proton-j/src/main/java/org/apache/qpid/proton/amqp/transport2/Flow.java, > > line 217 > > <https://reviews.apache.org/r/30379/diff/2/?file=843887#file843887line217> > > > > I'm not sure we should do this kind of validation here. This is really > > a semantic check, I would stick to just syntactic checks in the decode, > > e.g. if you get a String where you expect an int or something like that. > > > > I would put the sort of semantic check you have here in a separate > > method since it would be useful to be able to a) omit it at runtime for > > performance reasons, and b) it would also be useful to perform the same > > validation prior to encoding. > > rajith attapattu wrote: > I see your logic in having the semantic checks on it's own and being able > to use on both encoding and decoding. > > But when do you envision this to be called during decoding? I want to > understand what you meant by "omit it at runtime for performance reasons".
Presumably right after we perform the decode in the FrameParser, we can do an assert(performative.isValid()) or something like that. Likewise, just prior to encode we can do the same thing. - Rafael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30379/#review70627 ----------------------------------------------------------- On Feb. 2, 2015, 6:38 p.m., rajith attapattu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30379/ > ----------------------------------------------------------- > > (Updated Feb. 2, 2015, 6:38 p.m.) > > > Review request for qpid and Rafael Schloming. > > > Repository: qpid-proton-git > > > Description > ------- > > Skeleton code for a possible approach for codec improvements in proton-j. > > > Diffs > ----- > > proton-j/src/main/java/org/apache/qpid/proton/amqp/transport2/Flow.java > 388cbef > proton-j/src/main/java/org/apache/qpid/proton/codec2/DecodeException.java > PRE-CREATION > proton-j/src/main/java/org/apache/qpid/proton/codec2/Decoder.java d61fdef > proton-j/src/main/java/org/apache/qpid/proton/codec2/Encodable.java > PRE-CREATION > proton-j/src/main/java/org/apache/qpid/proton/codec2/POJOBuilder.java > c95844b > > proton-j/src/main/java/org/apache/qpid/proton/codec2/PerformativeFactory.java > PRE-CREATION > > proton-j/src/main/java/org/apache/qpid/proton/codec2/TransportTypesDecoder.java > a262760 > > proton-j/src/main/java/org/apache/qpid/proton/codec2/TransportTypesEncoder.java > c7c888c > proton-j/src/main/java/org/apache/qpid/proton/codec2/TypeRegistry.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/30379/diff/ > > > Testing > ------- > > > Thanks, > > rajith attapattu > >
