On Tue, 4 Oct 2022 05:07:13 +0600
NRK <nrk_AT_disroot.org> wrote:
Dear NRK,
> Another possibility is wrapping the integer inside a struct:
>
> typedef struct { unsigned internal_state; } GRAPHEME_STATE;
>
> the benefit of this is that the type GRAPHEME_STATE clearly states the
> purpose, whereas a `uint_least16_t` doesn't.
>
> Wrapping an enum into a struct is also a common trick to get stronger
> type-checking from the compiler; I don't think it matters in this case
> though, since the state is always passed via pointer.
>
> > and I want all of the semantics to be crystal clear to the
> > end-user.
>
> Other way of looking at it is that the state is an internal thing so
> the user shouldn't be concerned about what's going on behind the
> scene.
yeah, you bring up good points that I also thought of. What one should
not forget is that those shenanigans also complicate the use of FFIs.
I really originally thought that the state type would be used in more
than one place, but that's not the case. Enough meaning is given to it
by the name of the variable, so it's cool.
> The `(uint_least16_t)1` casts don't really do much since `int` is
> guaranteed to be 16bits anyways. But if you want to be explicit, you
> can still use `UINT16_C(1)`, which is shorter thus less noisy,
> instead of casting:
>
> - out->prop_set = in & (((uint_least16_t)(1)) << 8);
> + out->prop_set = in & (UINT16_C(1) << 8);
ah yeah, I always seem to forget about this macro, even though I use it
so often in the code. Fixed[0] now.
> I'd also return by value in these 2 functions. Expressions like these
> are more clear compared to out pointers:
>
> *s = state_deserialize(&state);
> state = state_serialize(*s);
I prefer to always pass structs by reference. Call me old-fashioned in
this regard.
Thank you for reviewing the changes, though. I really appreciate it! :)
With best regards
Laslo
[0]:
https://git.suckless.org/libgrapheme/commit/0aa5d262f8d0975341bcc60916e12044c7d64d0d.html
Received on Tue Oct 04 2022 - 08:29:01 CEST