WebOfTrust / keria

KERI Agent in the cloud
https://keria.readthedocs.io/en/latest/
Apache License 2.0
16 stars 26 forks source link

Remove unnecessary ser/des of simple primitive values #229

Open SmithSamuelM opened 2 months ago

SmithSamuelM commented 2 months ago

In many places in Keria, simple primitive values are deserialized into a Matter class and then immediately serialzed back to their serialization. This is unneccessary and in general an antipattern for how to use primitives. It is understandable to think of the Matter instances as the primitive but the Matter instance is so one can deserialize from a stream in order to apply some operation to the raw. Or similarly starting with a raw value serialize to a stream. But if one is just passing around Matter instances and never using the raw but simply des/ser/des everywhere then its a misapplication or anti-pattern.

for example: in testing_helper issueLegalEntityvLEI we have the following code

        vcid = iserder.ked["i"]
        rseq = coring.Seqner(snh=iserder.ked["s"])
        rseal = eventing.SealEvent(vcid, rseq.snh, iserder.said)
        rseal = dict(i=rseal.i, s=rseal.s, d=rseal.d)

Note that iserder.ked["s'] is already serialized in the format needed for putting into rseal but the code deserializes to Seqner(snh= and the immediately reserialized to .snh

Better would be

        vcid = iserder.ked["i"]
        rseq = iserder.ked["s"])
        rseal = eventing.SealEvent(vcid, rseq, iserder.said)
        rseal = dict(i=rseal.i, s=rseal.s, d=rseal.d)

or taking advantage of the .pre and .snh properties of SerderKERI

        rseal = eventing.SealEvent(iserder.pre, iserder.snh, iserder.said)
        rseal = dict(i=rseal.i, s=rseal.s, d=rseal.d)

Further taking advantage of namedtuple method ._asdict

        rseal = eventing.SealEvent(iserder.pre, iserder.snh, iserder.said)._asdict()

We refactor to one line of code

this pattern of unnecessary des/ser/des is repeated in numerous places especially for Seqner and Saider and Prefixer

Sometimes the antipatter is not obvious because there might be several function or method calls between the first des/ser and the final ser/des but no where does the raw get used nor any of the other methods of the Matter subclass other than to des/ser/des

In other cases it is necessary because the end serialization is as a concatenated list of Matter primitives for lmdb and the Matter instances are required to deserialize.

I think the latter above may have incented simpley using des/ser/des in other applications.

Although CesrSuber supports ser/des to a database entry of a single primitive (not concatenated) in most cases just using Suber with the serialized value will work. The use case for a single value is if the des will need to use the raw value or some other Matter methods besides ser. Then there is no point to using CesrSuber just use Suber.

Its only in the concatenated case that needs a way to des the concatenation and then CatCesrSuber is needed to parse out the primitives from the concatenation.