|
Take a switch to that guy. 
|
|
|
|
|
And then encase him in concrete. 
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
|
|
|
|
|
but that might break him.
"If you think it's expensive to hire a professional to do the job, wait until you hire an amateur." Red Adair.
nils illegitimus carborundum
me, me, me
|
|
|
|
|
The three above comments made my night...thanks 
|
|
|
|
|
You guys are very punny. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Future enhancement perhaps.
|
|
|
|
|
At times, when I have found things like this, I agree my first intent is to go find the person that wrote the code, but then, I sometimes stop and think, who REVIEWED IT and ALLOWED it into the code-base to start with?
The does of course mean that you have a review cycle as part of your policies...
Been to many places that don't, and then, yes, I run through the halls creaming for the head of the the programmer... and then use it as an example as to why they should implement code reviews 
|
|
|
|
|
The joke of this is: I know who has wrote this code... my boss...
|
|
|
|
|
RUN!
Run NOW!
But slap them first
|
|
|
|
|
Found due a code review at a big, big customer ... it's production code
if (object = nil) then
begin
object := NIL;
end;
Reallife is only for those people that cannot find friends in internet
|
|
|
|
|
Maybe objects in that language need to be shouted at in order to get the message.
|
|
|
|
|
Perhaps the original programmer was not so sure whether nil = NIL or not...
|
|
|
|
|
Do you hear me nil??
you are nil!!!
always remember it
|
|
|
|
|
At least is doesn't do anything harmful 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
|
You just have to see the upside.
As long as they are sitting at keyboards,
they aren't operating heavy machinery or driving a vehicle on the same roads as you or your loved ones.
-Rd
|
|
|
|
|
5! I love an optimist! 
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.
|
|
|
|
|
But they may be writing the code controlling the machinery 
|
|
|
|
|
Sadly true.
At least someone corrected the error in a later post.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
I almost choked. You have to pay me a new keyboard
|
|
|
|
|
Also, as a thumb rule, everything derived from object might be a class.
A while ago he asked me what he should have printed on my business cards. I said 'Wizard'.
I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.
|
|
|
|
|
I found the following class while debugging an application showing infrequent deadlocks:
template <class T>
class Protector
{
private:
CRITICAL_SECTION CS;
T data;
public:
Protector() { InitializeCriticalSection(&CS); };
~Protector() { DeleteCriticalSection(&CS); };
void operator=(const T& value)
{
EnterCriticalSection(&CS);
try { data = value; } catch(...){}
LeaveCriticalSection(&CS);
};
T& __fastcall operator *()
{
EnterCriticalSection(&CS);
T &value = data;
LeaveCriticalSection(&CS);
return value;
}
T* __fastcall operator ->()
{
return &data;
}
__fastcall operator T()
{
EnterCriticalSection(&CS);
T& value = data;
LeaveCriticalSection(&CS);
return value;
}
};
The class is used to wrap member variables supposedly for adding thread safety to them. For instance:
class foo
{
Protector<double> m_foobar;
}
It's something like an interlocked exchange based on RAII except for it doesnt make much sense. For me this appears to be cargo cult programming that does not bring any thread safety at all. My main concern are these lines:
EnterCriticalSection(&CS);
T& value = data;
LeaveCriticalSection(&CS);
return value;
This appears to be pure nonsense since the lock is acquired just in order to assign the a reference to data. (what on earth could change the address of data while in here?) The lock is then immediately unlocked and the reference returned. What is this supposed to protect? The reference can never change since &data can not change and the value in data is not protected since the lock is released immediately.
I think i'm going to get rid of this since it does'nt appear to serve any real purpose. Any other thoughts on that?
|
|
|
|
|
I think you're right, the Protector isn't protecting much, however can you relate the occasional deadlocks you're looking for to any of this code?
|
|
|
|
|
I suspect it is a deadlock based on the error report: "The program is freezing once a week". I have no hard evidence for this class beeing the cause of the problem except it's the only place in the program actually locking anything. I don't have more than that so i'm just looking for code that could potentially trigger a deadlock. (i. e. Unnecessary locks beeing acquired) If i'm lucky this was indeed the problem, if not there is at least fewer overhead if i remove the useless locks.
|
|
|
|
|
I think the problem is elsewhere. Changing this code will have some influence on the program behavior, if anything it most likely will reduce the frequency of deadlocks, making it even harder to pinpoint the real problem. I would not modify the code, except for adding observability (say logging to a file) until the deadlock is better diagnosed, then fix it and clean up whatever deserves cleaning up, but no sooner for those parts that may have overall influence.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
