|
A wee bit of commmenting would be nice, but other than that I cannot see a problem here.
As for porting to big-endian system: First of all, that will never happen. Secondly, there will be more places to fix if it did happen. Thirdly, the code is platform-restricted anyway (to Windows), so there is not a problem...
|
|
|
|
|
Why is that code platform-restricted to Windows?
I've seen DWORDs on many platforms, so there's really nothing here to suggest that.
|
|
|
|
|
Ah, but it was Windows that started it. Nyah, nyah...
As for the two other points, they are still valid...
|
|
|
|
|
Tom Delany wrote: event that the code is ever ported to a big-endian system, it would be completely broken.
No it wouldn't.
The code POSTED would work regardless of that.
As mentioned it is a TCP stream so it sequential. The switch statement is using ascii. The only way endianess would matter would be if the stream started to use a different character set. And if it did that it would fail if
1. The character system was not using a 8 bit lower representation of that matched ascii (UTF8 does where UTF16 does not.)
2. AND if the protocol changed.
And certainly if item 2 is true then all sorts of problems could result. Such as the rest of the code, following the switch, failing as well.
|
|
|
|
|
That's how you handle tags in ICC profiles, for example. It's a perfectly reasonable thing to do.
Tom Delany wrote: WE ARE DYSLEXIC OF BORG. Refutance is systile. Your a$$ will be laminated.
Still one of my favorite sigs on CP.
Software Zen: delete this;
|
|
|
|
|
No endian-sensitive code (which includes all multibyte chars) is "perfectly sensible".
|
|
|
|
|
'Sensibility' of a particular approach always depends upon the problem being solved. Making code endian-insensitive is necessary only if you plan on porting it between different endian environments.
Software Zen: delete this;
|
|
|
|
|
|
I suppose he didn't liked If - else- ifs...
|
|
|
|
|
It's sort of both... it'd be properly brilliant if the "verb =" line were commented...
|
|
|
|
|
Tom Delany wrote: This is either brilliant, or just bloody awful.
What is the fourth character of the protocol?
Tom Delany wrote: I'm coming down on the side of "bloody awful":
How would you refactor it?
|
|
|
|
|
In my eyes it is ugly, but one of the easies way to compare strings (up to 8 bytes) in a most efficient way.
If it's needed and documented ... ok.
I recently saw a very strange way to copy an object in C++:
- the class defines all its data member attributes within 64 Byte
- the data member attributes are ordered to use alignment efficently
- copying the class is done this way (or similar)
C64bitClass src, dst;
__int64 *pnSrc = static_cast<__int64*>(&src);
__int64 *pnDst = static_cast<__int64*>(&dst);
*pnDst = *pnSrc;
This really ugly, isn't it!
|
|
|
|
|
if (result != Common.ErrorMsg.ErrorNone)
{
this.Dispatcher.BeginInvoke((ThreadStart)delegate()
{
});
return;
} I found this in code that is guaranteed to be called on the dispatcher's thread. Sigh2.
Software Zen: delete this;
|
|
|
|
|
Someone probably planned on coming back and filling this in someday.
|
|
|
|
|
That explains why he copy/pasted it all over the place.
Software Zen: delete this;
|
|
|
|
|
I guess he liked spaghetti (code) with a side of Copy-Pasta.
Gryphons Are Awesome! Gryphons Are Awesome!
|
|
|
|
|
Procedural Object Oriented Programming (POOP).
|
|
|
|
|
Another guy here has his Special High-Interest Task list.
Software Zen: delete this;
|
|
|
|
|
Given you said it was replicated in multiple places, I can see only one possible perceived value -- to block this function from returning until after the dispatcher thread is finished executing something else. Such behavior isn't guaranteed, but it would work just often enough that I can easily see someone clueless thinking it does. Before totally writing it off, I'd look carefully at what's done with the dispatcher prior to executing this code.
As pointed out, this only makes sense if its run with an Invoke , instead of the BeginInvoke that's being used. Nevermind
We can program with only 1's, but if all you've got are zeros, you've got nothing.
modified 2-Apr-13 14:09pm.
|
|
|
|
|
It's a BeginInvoke call, which only blocks (very) briefly while the work item is queued to the dispatcher. The dispatcher in turn will execute a brief delay at some point executing the 'empty' work item.
Software Zen: delete this;
|
|
|
|
|
D'Oh! I wasn't paying attention and thought it was an Invoke call. Yeah, BeginInvoke has absolutely no value
We can program with only 1's, but if all you've got are zeros, you've got nothing.
|
|
|
|
|
Aaaahhh!, nothing like a do nothing instruction early in the morning...
|
|
|
|
|
I forget exactly what the purpose was, but I've seen something very similar done in WPF. The purpose was something like forcing the rendering to refresh. Though, I thought some "priority" (or something like that) was necessary for it to work.
I know, vague, but there could be a real purpose (that they apparently didn't leave a comment for).
|
|
|
|
|
WPF has the same constraint that you had under Win32 and MFC: you can only modify UI objects from the UI thread. I think the intent here was to promoted a UI change to the UI thread. Unfortunately, this code appeared in handlers that are already guaranteed to be called from the UI thread.
Software Zen: delete this;
|
|
|
|
|
That's a pretty special no-op.
As for using Invoke/BeginInvoke though, unless it's really really obvious that it can only be called in the dispatch thread (e.g. it's in an event handler or something), perhaps it used to be used from multiple threads, or the author couldn't be sure?
|
|
|
|