|
I guess this coder had seen the double check synchronisation pattern, e.g.
X getSynchronisedSingleCopyOfX(){
if(x == null){
lock(this){
if(x == null) x = new X();
}
}
return x;
}
... (commonly used in singleton instantiation) and didn't understand what the real benefit of the double check in that case was.
Or maybe he was just an idiot 
|
|
|
|
|
haah ai was laughing for about a minute, i remember now i was programming in c++ and i received the following message:"suspicious pointer conversion"
|
|
|
|
|
It can literally be replaced by 'return retval = flag'. If retval isn't local, changing it to 'return flag' might break something, though it's awful code if that's the case.
|
|
|
|
|
but the better code would be
if(flag) //skipping creating the ! result
return true;
return false;
[edit]
i thought it is better  ))))))))))))))))))))))))))))))))))))
[/edit]
modified 17-May-12 7:55am.
|
|
|
|
|
No, "return flag" is better than this. More words is worse if they add nothing.
|
|
|
|
|
Actually,
return flag could be wrong. flag could be a non-Boolean value (e.g. 0 or "" or [not so well known so a major gotcha] '\n' which have a Boolean equivalent of false; whereas most other values have a Boolean equivalent of true). So, to return false for ! flag and true for flag, you should use
return ! ! flag; . This is a useful trick: the 2nd
! converts flag (in this case) to a Boolean and negates it (true -> false and false -> true) then the 1st
! negates it back to what its Boolean equivalent was.
|
|
|
|
|
Good point, although using a variable name of 'flag' for something which wasn't a boolean would be deserving of headline mention in this forum!
Can't you just cast to boolean instead of using !!?
|
|
|
|
|
BobJanova wrote: Can't you just cast to boolean instead of using !!?
Yes, you could. Casting could be done using
return Boolean(flag); Note that
return new Boolean(flag); would return a Boolean object whereas without the new keyword, you'd be casting to a Boolean primitive. Greater clarity could be effected by using the triadic operator:
return flag ? true : false; or you could use short circuiting, viz
return flag && true; but using ! ! flag is more succinct.
(I noticed yesterday a piece of jQuery that used !1 for false - I wouldn't go that far in obfuscation).
|
|
|
|
|
IMHO, return flag; is still better than this.
|
|
|
|
|
oh f***, i didnt notice the stupidity )))) sorry guys
|
|
|
|
|
Its ok. If only this realization would have came to the original developer of this code
|
|
|
|
|
Yes, you must not change the sacred elephant code or you will anger the coding spirits. 
If they fear making changes to the code then they need unit tests to prove that the code still works after changes. Also, if the code is so brittle that it may break after a small improvement then it needs reviewed and replaced with better code.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Just saw this:
public interface IInterface
{
//some interface
}
internal SomeClass : IInterface
{
}
public class PublicAPI
{
public void PublicFunc(IInterface i)
{
if(!(i is SomeClass))
{
throw new Exception("External implementation of IInterface is not allowed!!"); //what a crap??
}
}
}
why on earth would you expose an interface if you restrict it to one single concrete implementation?
|
|
|
|
|
Nice one... looking forward to a possible explanation
(yes|no|maybe)*
|
|
|
|
|
He must be sleeping  while coding 
Explanation:
Currently we allow only internal implementation, in future we may allow external as well, so we have interface.
Does it make any sense??
|
|
|
|
|
Ankush Bansal wrote: Does it make any sense??
Er... no... but what does
(yes|no|maybe)*
|
|
|
|
|
|
Perhaps it is some kind of Monomorphism implementation using interfaces
|
|
|
|
|
|
That is bizarre. I wonder what was going through the original coder’s head?
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
His ass?
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
|
|
|
|
|
Skydiving with a malfunctioning parachute?
|
|
|
|
|
One reason I can think of is to reduce the impact of a later change.
So, you have IAnimal, implemented by both Duck and Dog. It would be useful to keep collections of IAnimal so you can hold both your Ducks and Dogs and perform IAnimal operations on them.
However, suppose you can't get to implementing Dog yet, but you know it will exist in the future. You might code toward a known future objective that doesn't appear to be of any use yet.
"Why not make IAnimal an internal class so it isn't visible outside the assembly", you ask. Well, that interface and the classes that implement it may be required in multiple assemblies.
|
|
|
|
|
Yeah, that sounds logical. The original coder really should have added comments to explain what he was doing and why. Unfortunately, those kind of comments are all too rare. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
I think you are missing a point here![Rose | [Rose]](https://www.codeproject.com/script/Forums/Images/rose.gif) .
The issue is not that they have exposed an interface say IAnimal and they only have Duck implementation, and Dog's is pending. The issue is they throwing an exception if concrete impl of IAnimal is external (even if its else's Duck).
Point being why to create a false API? Why to show that API can handle any IAnimal but would throw if animals are external.
Why can't an IAnimal be external? why can't someone else's Duck be processed?
I personally believe that if any point there is a need of such check then either one should make concrete impl of Duck public and take that as input in func or one can have a public proxy for Duck.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
