Hi Lukasz, 

I think the code wouldn't even get so bad ... and regarding the "optional" flag 
... from a parser/serializer perspective it doesn't matter if a field is 
"double-optional" ... 

And I agree with your assumption that we should postpone work on this till 
after 0.9.0. And while at it I would love to merge the "pojo" and the "io" 
templates together that we no longer have these stupid "io" packages in Java. 
In all other languages the model and the code for the parsing and serializing 
are in one place. I only split things up for java as this was the first 
language and I didn't want to have "discussion" with clean-code-fundamentalists 
at my old employer ;-)

Chris


-----Ursprüngliche Nachricht-----
Von: Łukasz Dywicki <[email protected]> 
Gesendet: Mittwoch, 25. August 2021 14:43
An: [email protected]
Betreff: Re: AW: [DISCUSS] Change the way we use "optional"

I think I do follow your point. I don't mind getting it implemented in a way 
which just works.

Maybe below example is not perfect, but shows two optional blocks nested under 
each other.
[type DataBlock [uint 16 'size']
  [optional 'size > 0'
    [implicit uint 32 'length' 'COUNT(data)' ]
    [optional 'length' > 0'
      [array int 8 'data' count 'length'     ]
    ]
  ]
]

If we end up with IF condition for each field then generated code will look a 
bit worse, but we will survive. Not sure how "bad" generator templates will 
get, but that's another story.

Best,
Łukasz


On 25.08.2021 14:24, Christofer Dutz wrote:
> Hi Lukasz,
> 
> don't understand me wrong ... I would adjust the parsers to do that ... it 
> would just be the way our generator-internal-model would keep track of it.
> 
> Problem is that for optional fields I need to generate the code 
> differently (pointers to simple types instead of direct variables) So the 
> "optional" would simply ensure the code is generated respecting that.
> 
> Chris
> 
> 
> -----Ursprüngliche Nachricht-----
> Von: Łukasz Dywicki <[email protected]>
> Gesendet: Mittwoch, 25. August 2021 14:14
> An: [email protected]
> Betreff: Re: [DISCUSS] Change the way we use "optional"
> 
> I see a valid point to get optional block since way pointed by Ben causes 
> creation of multiple wrappers.
> 
> Assuming that we have nested optional fields:
> [type OptionalBlock
>   [implicit uint 32 'length' 'COUNT(data)'   ]
>   [array int 8 'data' count 'length'         ]
> ]
> [type DataBlock [uint 16 'size']
>   [optional OptionalBlock 'size > 0'         ]
> ]
> 
> After changes it would be:
> [type DataBlock [uint 16 'size']
>   [optional 'size > 0'
>     [implicit uint 32 'length' 'COUNT(data)' ]
>     [array int 8 'data' count 'length'       ]
>   ]
> ]
> 
> I think that getting with optional flag under the hood will serve us near 
> term. However long term solution should introduce optional as a block into 
> our parser and generators. This will allow us to nest multiple optional 
> sections. Otherwise we will shift problem just one level down.
> 
> Since this is not a strong requirement for 0.9 we might plan this after we 
> ship nearest release.
> 
> Best,
> Łukasz
> 
> 
> On 25.08.2021 13:05, Christofer Dutz wrote:
>> Hi,
>>
>> as the discussion just came up on Slack ... taking it where it 
>> belongs ... on the list ;.)
>>
>> So the problem was, that it's difficult to have anything optional that is 
>> not a "simple" field.
>>
>> My Idea would be to change the way "optional" is used and to convert it into 
>> a wrapper element. It would just contain an expression and contain normal 
>> fields inside.
>>
>> So an
>>
>> [optional uint 8 "hurz" "some expression"]
>>
>> Would bebome:
>>
>> [optional "some expression"
>>                [simple uint 8 "hurz"] ]
>>
>> This way we could even wrap multiple elements inside the optional condition 
>> and all of our types.
>> In general it would simply map to an if expression.
>>
>> I doubt the changes would be very significant. We would need to add an 
>> "optional" flag to the internal field types and set that to false in the 
>> general case but to true inside an optional field.
>>
>> Chris
>>

Reply via email to