|
|
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.
|
|
|
|
|
Being a bit of a devils' advocate here.
I think this code definitely require a better message in the exception or description in the code.
However
Perhaps the programmer want to prevent the class from being inherited.
This is why he made it internal.
He could have make it public and sealed and as I see it get the same effect.
Sometime when you define an api though, it might not be such a bad idea to keep everything in interfaces.
This way the library can be modified without impacting the Interfaces that have been exposed previously.
|
|
|
|
|
Has anyone ever come across and entire project where every class, every function is a static, for no good reason at all. Just a ton of statics and delegates and that is it. It is a pretty large project.
|
|
|
|
|
Sounds ideal to me; why use OOP when you don't need it?
|
|
|
|
|
|
|
Written by a former VB6 programmer? 
|
|
|
|
|
Why, yes, I have seen a project with everything that could be made static was made static, but they did have a reason. When I asked why all those methods were static, I was told "so that they can be called by an outside program". I didn't say they had a good reason. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Not everything, but really a lot: WPF.
|
|
|
|
|
One place I interviewed at a few years ago showed me their code -- every class was a Singleton!  -- I didn't ask for the job.
|
|
|
|
|
Once upon a time
Manager: Show me your design diagrams.
Developer: Here they are but I am still working on it.
Manager: Where is the class for our front page.
Developer: (confused) That would be a static page so we dont need the classes.
Manager: OK now you work on task 2. i will get this one done by developer 2.
Later that day
Developer2: Is it complete
Manager: yes.
Developer2: where are the relationships between classes?
Manager: hmm, well all these classes and functions are static so we don't need them.
Developer2:WHAT?
Manager: yes now go and do it before I sack you.
|
|
|
|
|
My boss recently got the habit of making a 'helper class' for just about anything. Each helper class has one shared (static) function that is usually called in just one place in the code. Even the helper classes have helper classes and each class is a seperate file sometimes even in seperate folders...
I am guessing he does this because classes now have a 'single responsibility'. Mixing up 'single method' and 'single responibility' is a common mistake I think. It especially doesn't hold true if all methods are shared (static). I'm also not about to argue about it with him. If he comes up with this kind of stuff you can imagine what kind of programmer he is: one that simply doesn't understand.
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
Shocking!
public class SysAdmin : Employee
{
public override void DoWork(IWorkItem workItem)
{
if (workItem.User.Type == UserType.NoLearn){
throw new NoIWillNotFixYourComputerException(new Luser(workItem.User));
}else{
base.DoWork(workItem);
}
}
}
|
|
|
|
|
Why use repeaters/datalists/grids when you can do it via a foreach loop and throw the whole string on a div.innerHTML? 
The item template never changes, and the guy keep doing it via code. 
Am i unlucky?
Look.
string html = "<table><tr>";
int cnt = 1;
foreach (DataRow reg in Data.Rows)
{
if (cnt > 3)
{
html+= "</tr><tr>";
cnt = 1;
}
html += "<td height='210px' valign='top' align='left'>" +
"<table align='left' height='100%' width='239px'>" +
" <tr>" +
" <td valign='top'>" +
@" <a class="Registroclick" href="javascript: void(0)" idseg="0" idcat=" + reg["some reg"].ToString() + " endereco='" + destpage+ "?idcat=" + reg["some reg"].ToString() + "'><img src='" + vid+ "destaque/" + reg["some reg"].ToString() + "' width='235' height='136' border='0'/></a><br/>" +
@" <a class="Registroclick" href="javascript: void(0)" idseg="0" idcat=" + reg["some reg"].ToString() + " endereco='" + destpage+ "?idcat=" + reg["some reg"].ToString() + "'>" + reg["some reg"] + "</a><br/><br/>" +
" </td>" +
" </tr>" +
"</table>" +
"</td>";
cnt++;
}
html += "</tr></table>";
divHtml.InnerHtml = html;
That's my bro, there's an classic asp tag to use in asp.net? Maybe he needs it.
It's a ascx with 150 lines. 100 lines on this method.
Also StringBuilder() and String.Format() seems to be useless.
Now imagine out how to update this layout;
return true;
|
|
|
|
|
Maybe he just wants tables, not the random crap (not working in some browsers) you can get from using an ASP control.
|
|
|
|
|
I'm reading a 790 line SQL sproc, a lot of poor coding peppered with extrainious begin/end statements. It starts early on with a "begin tran" lots of poor inserts and updates. Looks like SQL 2000 code, no try/catch logic, then the kicker:
End
IF @@ERROR <> 0
-- error statements using the first needed begin/end block
Else
Commit Tran
End
That was the first reference in the whole novel to @@ERROR! (Code created for Msft)
For those who don't know SQL, that "End" before the "IF" FORCES it to always try to Commit the transaction, no matter how many errors the user has hit by now.
|
|
|
|
|
I have seen something like that. A big problem if it's in a high security thing.
return true;
|
|
|
|
|
Sounds like it was written by someone who doesn't really know SQL. They are only worth keeping around if they can be trained to do it the right way. If they argue that their way is perfectly fine, then chuck them out the door.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
One developer at work like to do things the hard way.
protected void ProcessDropDown(DropDownList myCombo, string myValue)
{
foreach (ListItem myItem in myCombo.Items)
{
if (myItem.Value == myValue)
{
myItem.Selected = true;
break;
}
}
}
All this just to select an item on aspx pages, should i throw him by the window?
And yes, this method is used only on the .cs file. So why use protected and do this thing? My eyes still hurt.
return true;
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
