On Mon, 20 Dec 2021 at 18:40, Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
On 12/20/21 10:14, Francisco Casas wrote:
December 17, 2021 8:13 PM, "Zebediah Figura (she/her)" <zfigura(a)codeweavers.com> wrote:
I think the assert is a bit superfluous.
On the other hand, when this kind of code pattern comes up, I tend to prefer using a helper, along the lines of
else if (!strcmp(name, "GatherGreen")) { add_gather(HLSL_RESOURCE_GATHER_GREEN, 1, ...); }
which avoids that kind of implicit agreement.
I am more inclined to just remove the assert. I must confess that I don't like jumping from function definition to function definition too much, so I prefer not to make new functions unless they will be called in two or more different places.
But if you insist I will add the helper, and not follow that policy of mine too much. In that case I think it would also make sense to add helpers for Load and Sample too. This since the code for these methods is fairly similar, so I think it would be better to keep them close, also for the sake of consistency.
I don't personally mind large functions, but on the other hand, the code style we use in wined3d tends to be relatively liberal with helpers (as long as they're well-defined, I guess.)
It's a minor thing, really, just something that struck me as a possible improvement.
For what it's worth, the idea there is that well-defined helpers provide information about the higher level intent and structure of the code.