|
I just refactored this:
if (requiresLocking)
{
Monitor.Enter(lockerObject);
}
if (requiresLocking)
{
Monitor.Exit(lockerObject);
}
Where requiresLocking is a variable passed in to the function.
Into this:
object thisLocker = new object();
if (requiresLocking)
{
thisLocker = lockerObject;
}
lock (thisLocker)
{
}
I am afraid that the original version was my own code and I am still not happy with the modified version.
Second thoughts: on looking at requiresLocking it will be true except in rare and unusual conditions (for example the application or windows is crashing.) In those conditions I can accept the extra delay of acquiring a lock 
I changed it to:
lock (lockerObject)
{
}
|
|
|
|
|
Member 2053006 wrote: object thisLocker = new object();
I hope that isn't a local variable. (It needs to be a field.)
|
|
|
|
|
How about this:
public void OriginalMethod(bool requiresLocking, object otherParams)
{
if (requiresLocking)
{
lock (lockerObject)
{
LotsOfCodeMovedToNewMethod(otherParams);
}
}
else
{
LotsOfCodeMovedToNewMethod(otherParams);
}
}
|
|
|
|
|
I would not do that. Conditional locking controlled by a parameter would introduce a nasty possibility for bugs. If this method is called from many different places it would be hard to ensure that requiresLocking is indeed set to the right value. If there is a way to determine this in the method itself we would not need this parameter.
If the method workes with data that is shared between threads, then it should always be locked. Conditional locking very probably so little a benefit that it's not worth the possible bug hunt.
I'm invincible, I can't be vinced
|
|
|
|
|
Your last version is the simplest and probably the best. Conditional locking with the help of a boolean parameter would only introduce an external source of bugs while (probably) bringing only unnoticably more performance.
What you really should take a look at is the 'Lots of (ahem, good) code here' part. Holding a lock for a longer time may reduce the benefits of working with several threads to zero. You may end up with the performance of a single threaded application. Breaking up the code, identifying smaller critical sections and using several short locks in those locations may help a lot.
I have worked on my own UI. There is an application thread, which of course often makes changes to the controls or their properties, and there is a rendering thread to draw the UI. Not synchronizing those two threads leads to ugly graphical effects when the rendering thread catches the application thread in the middle of some changes. Now I could simply lock the entire control tree when rendering or when making changes to the UI. This would work, but one thread would come to a complete halt when the other one is doing something.
Instead, I never lock the entire tree, but only the one control that is currently going to be rendered or changed. One huge lock has been replaced by many brief locks. This way the two threads collide far less often (only when they happen to access the same control) and if they do, then the wait for the other thread is only brief. Locking is always a manner of 'as much as needed and as little as possible'.
I'm invincible, I can't be vinced
|
|
|
|
|
Very good explanation and clear example 
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
don't forget to check that your current process owns the lock after setting it - another process may have locked it between you checking if its locked and you locking it
|
|
|
|
|
We have a tool to generate HTML reports. And in this generated HTML we have links in between sections. Those links are very useful for jumping in between sections. However, When the HTML is saved/moved to another location, all the links started to fail.
Simple inspection of the HTML source revealed the problem. All the links used were like this:
<a href="file:///C:/Documents%20and%20Settings/krumia/Local%20Settings/Temp/20120301-00035.html#ta026A3538">some text</a>
They used freaking absolute paths. Could have just used the section anchor #ta026A3538.
Now think of the disadvantages: (a) As soon as you save the freaking file anywhere else it will be useless. (b) File size increases because of long absolute paths.
Peace, ye fat guts!
|
|
|
|
|
That's one way to license your product. Won't work on other machines.
|
|
|
|
|
Interesting how the hard coded URL has the word krumia in it! 
"You get that on the big jobs."
|
|
|
|
|
he he 
I replace my actual username with krumia.
Peace, ye fat guts!
|
|
|
|
|
Haha, I remember one government site. That entire site was developed in the local windows based PC and when they upload it to the linux server all the freaking path was absolute "C:/Program Files/PHP/WWW/....."
Mentally disorder will do better work than that one
|
|
|
|
|
Sigh...read this (again):
http://www.cs.utexas.edu/~EWD/ewd02xx/EWD215.PDF[^]
Note that Dijkstra is speaking specifically about "higher level" languages. Assembly level code will never dispense with the goto statement, because it maps directly to the machine JMP instruction. Very efficient. But this can't justify their use in a higher-level language.
Hhigher-level programs should represent complex logic through abstraction. In OO languages, complexity in a method is usually an indication that the object model should be refactored...without using goto.
|
|
|
|
|
|
Well, that was the biggest bunch of tripe spouted as an erudite document that I've read since college. That's worth the hall of shame all by itself.
It's stored as images when HTML text would be much more readable, manageable, and condensed in storage costs to boot.
This guy is using circular logic to justify not supporting the GOTO command, which I found ironic.
They copyrighted something that anyone should be ashamed to claim as their own.
Spell-checkers have been around forever, I'd have thought a professor might have found one by now.
I have to admit that I started hating GOTOs several decades ago when I read one tome without a single comment in it and as near as I could tell about a third of the code was dead code. I'm predisposed to like a document that advocates this concept and I didn't see one valid argument given.
Says it produces errors, which I've seen firsthand, but doesn't explain why they are produced.
The only reason you'd need to re-read it, is if you are searching (again) for its logic.
I kind of liked the original poster posting it like it is a document worth reading, but puts it in the hall of shame. Eliminating GOTO statements deserves a well written document. That wasn't it.
|
|
|
|
|
Well, that "tripe" was written by Dr. Dijkstra who is considered to be the father of programming languages and much of the framework of what we call computer science today. If you actually read the beginning it was also written in 1968, on a typewriter. You ever seen one, child? There is no spell checker on an IBM Selectric.
I also have seen developers today utilize GOTO: in C# which makes me shudder!
|
|
|
|
|
Just started working on some objective C code today and been giggling quite a bit. I thought I'd post this little gem.
NSString *dateCompletedTitle = [[NSString alloc] initWithFormat:@"%@", NSLocalizedString(@"PDFCompletedKey", @"")];
Creating a string to return a localized string that's formatted as a string by creating another string. Even better, the application only supports one language, English.
And if you did localize it to support a number of languages, the translator would have no idea of what PDFCompletedKey was as there is no comment.
Life's too short to refactor this project.
"You get that on the big jobs."
|
|
|
|
|
One of the gems I keep finding in a (former) coworker's code:
CString string;
string.Format(_T("some text"));CString::Format() does 'C' printf() style formatting. Why go through all that just to initialize a string to a constant value? 
For the non-MFC folks in the audience:
CString string(_T("some text"));is the preferred way to do this.
Software Zen: delete this;
|
|
|
|
|
There's a slight memory management issue at work here though.
If using iOS ref counting (not ARC), the variable you're showing is retained, whereas the localized string answer would be autoreleased.
So it's a fancy way of doing [NSLocalizedString(...) copy].
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
Honestly, no matter how bad or well written it is, all objective C code looks like a coding horror to me. But I suppose this looks worse then most.
|
|
|
|
|
SELECT MONTH + MONTH(DATEADD(qq,-1,DATEADD(qq,DATEDIFF(qq,0,@Today),0))) - 1
FROM table1 t JOIN (SELECT TOP 3 row_number() over (ORDER BY fieldA) as MONTH FROM AnyTable) B on 1 = 1
The first MONTH is actually the aliased field name from B. Why alias a field using the same name as a built-in function? Very confusing to follow. Although, unfortunately, it does actually work.
On the plus side, joining to AnyTable using Top 3 Row_Number() is a clever way to guarantee 3 rows. Guess I could also put this in the Clever Code but it was written by the same dev.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
ryanb31 wrote: Although, unfortunately, it does actually work
Its an unfortunate issue that you posted the code saying unfortunately its work.
Did you loose your job because of this unfortunate event?
|
|
|
|
|
Unfortunately, he didn't lose the unfortunate job.
|
|
|
|
|
It's bad practice to name something using the same name as a built in function, even if it does work.
|
|
|
|
|
The bad practice is failing to write the statement explicitly.
Why change MONTH when it's meant to be 'month'?
The shame is not using "SELECT B.[MONTH] + MONTH..."
Yet, even that is minor.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
