Re: [wined3d] drawprint tidyup initilization of strided data.
+#define SET_STRIDED (_usage, _name) \ +case _usage: \ +strided->u.s._name.lpData = data; \ +strided->u.s._name.dwType = element->Type; \ +strided->u.s._name.dwStride = stride; \ +break; + +#define SET_STRIDED_(_usage, _name) \ +case _usage: \ +strided->u.s._name.lpData = data; \ +strided->u.s._name.dwType = element->Type; \ +strided->u.s._name.dwStride = stride; \
This is pretty ugly: the resulting code doesn't look like C. In fact, it looks rather magical.For example, why pass in _usage as a param to the macro, just to get rid of the case _usage: which you actually want outside the macro for readability. Also, accessing the magic strided, data, element, stride directly is very weird, and problematic. The casual user wouldn't know what hit him :) I say let it be as is. If you really want the macro, something like this would be better: +#define SET_STRIDED_(_strided, _name, _data, _element, _stride) \ +_strided->u.s._name.lpData = _data; \ +_strided->u.s._name.dwType = _element->Type; \ +_strided->u.s._name.dwStride = _stride; -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
--- Dimi Paun <dimi(a)lattica.com> wrote:
+#define SET_STRIDED (_usage, _name) \ +case _usage: \ +strided->u.s._name.lpData = data; \ +strided->u.s._name.dwType = element->Type; \ +strided->u.s._name.dwStride = stride; \ +break; + +#define SET_STRIDED_(_usage, _name) \ +case _usage: \ +strided->u.s._name.lpData = data; \ +strided->u.s._name.dwType = element->Type; \ +strided->u.s._name.dwStride = stride; \
This is pretty ugly: the resulting code doesn't look like C. In fact, it looks rather magical.For example, why pass in _usage as a param to the macro, just to get rid of the case _usage: which you actually want outside the macro for readability.
Well, when the #if 0's are removed the whole lot should fit on a screen so you can see the macro and it's usage quite clearly and it no longer looks like magic.
Also, accessing the magic strided, data, element, stride directly is very weird, and problematic. The casual user wouldn't know what hit him :)
It's less problematic since if forces the code into a strict layout, why have extra paramiters when there going to be identical? Every opportunity for a typo is an opportunity for a bug, less code less bugs, especially when it's being changed by the 'casual user'
I say let it be as is. If you really want the macro, something like this would be better:
Well, if I leave it as be, when I want to change the structure I have to change it ten times instead of once, that's ten times the opportunity for bugs. It's just my opinion that when you reduce the amount of repetition in the code you reduce the possibility of error, and when the entire function is viewable without scrolling that adds to the readablity of the code.
+#define SET_STRIDED_(_strided, _name, _data, _element, _stride) \ +_strided->u.s._name.lpData = _data; \ +_strided->u.s._name.dwType = _element->Type; \ +_strided->u.s._name.dwStride = _stride;
-- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
___________________________________________________________ Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com
From: "Oliver Stieber" <oliver_stieber(a)yahoo.co.uk>
Well, when the #if 0's are removed the whole lot should fit on a screen so you can see the macro and it's usage quite clearly and it no longer looks like magic.
No, the _usage_ will always look like magic. It will magically use and modify var from the local context. No other C construct does that. This is (one of) the problem with macros. It's best if their usage is not very different then a regular C construct. Principle of least surprise and all that.
Also, accessing the magic strided, data, element, stride directly is very weird, and problematic. The casual user wouldn't know what hit him :)
It's less problematic since if forces the code into a strict layout, why have extra paramiters when there going to be identical? Every opportunity for a typo is an opportunity for a bug, less code less bugs, especially when it's being changed by the 'casual user'
Because it makes is clear what is going on there. -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
After looking at glibc nss code for a while, and also trying to understand how wined3d/device.c code updates pp* variables (it constructs the varname from a string pattern, and updates it), it is my opinion that macros are pure evil, and inline functions should be used everywhere instead. ..that's just my opinion as the "casual observer".
--- Dimi Paun <dimi(a)lattica.com> wrote:
Also, accessing the magic strided, data, element, stride directly is very weird, and problematic. The casual user wouldn't know what hit him :)
It's less problematic since if forces the code into a strict layout, why have extra paramiters when there going to be identical? Every opportunity for a typo is an opportunity for a bug, less code less bugs, especially when it's being changed by the 'casual user'
Because it makes is clear what is going on there. I tend to wonder why code is duplicated when it is, expecting to find out that one instance works in a different way from the other. So for me, the macro is clearer, but since it's not clear to everyone the patch should be dropped and I'll make more verbose tidyups in future if at all.
-- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.
___________________________________________________________ How much free photo storage do you get? Store your holiday snaps for FREE with Yahoo! Photos http://uk.photos.yahoo.com
participants (3)
-
Dimi Paun -
Ivan Gyurdiev -
Oliver Stieber