You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by Christofer Dutz <ch...@c-ware.de> on 2021/08/25 11:05:31 UTC
[DISCUSS] Change the way we use "optional"
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
AW: [DISCUSS] Change the way we use "optional"
Posted by Christofer Dutz <ch...@c-ware.de>.
And for the record ... I'm totally +1 on this ;-)
-----Ursprüngliche Nachricht-----
Von: Christofer Dutz <ch...@c-ware.de>
Gesendet: Mittwoch, 25. August 2021 13:06
An: dev@plc4x.apache.org
Betreff: [DISCUSS] Change the way we use "optional"
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
AW: AW: [DISCUSS] Change the way we use "optional"
Posted by Christofer Dutz <ch...@c-ware.de>.
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 <lu...@code-house.org>
Gesendet: Mittwoch, 25. August 2021 14:43
An: dev@plc4x.apache.org
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 <lu...@code-house.org>
> Gesendet: Mittwoch, 25. August 2021 14:14
> An: dev@plc4x.apache.org
> 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
>>
Re: AW: [DISCUSS] Change the way we use "optional"
Posted by Łukasz Dywicki <lu...@code-house.org>.
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 <lu...@code-house.org>
> Gesendet: Mittwoch, 25. August 2021 14:14
> An: dev@plc4x.apache.org
> 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
>>
AW: [DISCUSS] Change the way we use "optional"
Posted by Christofer Dutz <ch...@c-ware.de>.
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 <lu...@code-house.org>
Gesendet: Mittwoch, 25. August 2021 14:14
An: dev@plc4x.apache.org
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
>
Re: [DISCUSS] Change the way we use "optional"
Posted by Łukasz Dywicki <lu...@code-house.org>.
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
>