|
...to remind myself of my thoughts when I quit last time, but this comment takes the cake:
; DO SOMETHING, I DON'T KNOW WHAT, BUT DO IT HERE..
|
|
|
|
|
I was recently told that REM stood for REMark and did the same as ' in VB or // in C#... I was very tempted to put something like REM - Losing My Religion above every Class I made
It's an OO world.
|
|
|
|
|
Few interesting comments :
int i=10; //Don't forget
//DO NOT REMOVE THIS COMMENT. REMOVING IT WILL AFFECT THE BUILD PROCESS
-
Bits and Bytes Rules!
10(jk)
|
|
|
|
|
I hate comments that are there just because 'code should be commented'...
I see stuff like
Public Sub GetInfoFromDataBase(connString As String)
conn.Open
If dataRow("someColumn").Value IsNot DBNull.Value AndAlso dataRow("someColumn").Value IsNot Nothing Then
End If
End Sub
And then when there really is a whole lot of unreadable code no one commented it...
It's an OO world.
|
|
|
|
|
Agreed. I really hate it when, in C#, someone just puts the method name where there is supposed to be a summary of what the method does.
public void MethodName()
{
}
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
C# guidelines. Method names should be descriptive. Why would you need comments?
|
|
|
|
|
If you have a nice tool like EA, you can put the comments at the top of methods and have those propagate into an API document. Who doesn't like a good API doc?
|
|
|
|
|
It took me a bit longer than I'd like to admit to detect the sarchasm.
|
|
|
|
|
But in the end, you did detect it.
|
|
|
|
|
I am but a naive human being, and it's going to be my downfall.
|
|
|
|
|
Yes, the method names should be descriptive. In my experience the ones who don't use descriptive method names also don't bother to write good comments. Also, sometimes, even a good method name can use a little extra description now and then.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Ah... Captain Obvious... finally we meet...
(yes|no|maybe)*
|
|
|
|
|
That (well, the first part) is what you get with coding standards that mandate comments (and function headers, referring to the subthread above). Comments should be there to explain something non-trivial (i.e. that a two second glance at the code won't give you), imo – a summary of a multi-line complex piece of logic, or an explanation as to why you want to do what the code is doing.
|
|
|
|
|
|
well, I would agree classes first of all should be easy to use, rather than easy to design and implement. The whole idea is to encapsulate the petty details so the user hasn't to worry about them.
Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum
Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.
|
|
|
|
|
Well, the implementation now looks like this...
Dim list As New ListInterfaceClass
list.AddToList(New Example1)
list.AddToList(New Example2)
list.AddToList(New Example3)
manager.AddToManager("String1", list)
manager.AddToManager("String2", New Example4)
The problem here is that I cannot add "String1" to the ManagerClass twice.
It should look like this according to my boss...
manager.AddToManager("String1", New Example1)
manager.AddToManager("String1", New Example2)
manager.AddToManager("String1", New Example3)
manager.AddToManager("String2", New Example4)
Really, how much work does this save the user? 4 words or 20 characters? Most is IntelliSence and if the list gets bigger it actually has less characters!
More importantly, to get the result my boss wants I should have the ManagerClass depend on the ListInterfaceClass while they are now completely ignorant of each other.
Microsoft did not put its Connection, Command and Adapter Classes into one Class to make it easier for the user either, did they? Perhaps ORM tools do something like that, but if that is how it is done then I guess I should also make some wrapper ListManagerClass to encapsulate the ManagerClass and ListInterfaceClass rather than changing the current ManagerClass
P.S. I did not really name my classes 'ManagerClass' and 'ListInterfaceClass', that WOULD make it hard on the user...
It's an OO world.
|
|
|
|
|
I see. There isn't that much of a difference from the caller's point of view after all.
You could take advantage of the params keyword, and accept the following:
manager.AddToManager("String1", New ListInterfaceClass(New Example1, New Example2, New Example3))
manager.AddToManager("String2", New Example4)
or
manager.AddToManager("String1", New Example1, New Example2, New Example3)
manager.AddToManager("String2", New Example4)
I might go for the latter.
Luc Pattyn [Forum Guidelines] [My Articles] Nil Volentibus Arduum
Please use <PRE> tags for code snippets, they preserve indentation, improve readability, and make me actually look at the code.
|
|
|
|
|
That is not a bad idea at all!
Of course it would be:
manager.AddToManager("String1", New ListInterfaceClass({New Example1, New Example2, New Example3}))
Your second option would once again require a dependency between ManagerClass and ListInterfaceClass . It is probably going to be like that anyway, because my boss wants it, but that would result in having to rewrite or at least recheck the ManagerClass once the ListInterfaceClass changes or is made obsolete. And that happens a lot in our company...
Besides, I recently showed something like that to my boss:
Dim something as New Class4(New Class3(New Class2(New Class1)))
His reply was "This looks really weird, did you just make that up? In all my seven years of experience I have never ever seen anything like that and Microsoft does not do that for sure!" then he started yelling and saying "You read some articles on the internet and think you know it all, but I have seven years of experience! You have obviously not understood anything if you make code like that!" I was speechless....
Maybe I posted this thread in the wrong forum. Although the 'less code is better' mentality is still something to be ashamed off. At least the way it is currently implemented (I know of a much worse example, but that would be to much to type here)
It's an OO world.
|
|
|
|
|
Naerling wrote: Of course it would be:
manager.AddToManager("String1", New ListInterfaceClass({New Example1, New Example2, New Example3}))
have you looked at the ParamArray keyword? It allows you to declare a function that takes a variable number of parameters and handles them as an array... so you don't need no curly braces
|
|
|
|
|
That is pretty nice!
Although ParamArray is still not what my boss wants. Simply passing Classes as parameters all of a sudden seems not done anymore :???: My boss says it is to confusing for new developers.
He now wants to change the above code alltogether, making it Generic and creating the Class that should be added using Reflection.
Code would look like:
manager.AddToManager(Of ListInterfaceClass)("String1")
According to him this is much more readable than:
manager.AddToManager("String1", New ListInterfaceClass)
I don't think anything can save me anymore, not even ParamArray
It's an OO world.
|
|
|
|
|
I've been wondering for a long time and I still do: why can't people learn elementary logic rules? Are they really that hard? For example, I constantly meet code snippets like this:
if (tabIndex == 0)
{
panelGeneral.Visible = true;
panelTolerances.Visible = false;
btnProjSetNext.Enabled = true;
btnProjSetPrev.Enabled = false;
}
else
{
panelGeneral.Visible = false;
panelTolerances.Visible = true;
btnProjSetNext.Enabled = false;
btnProjSetPrev.Enabled = true;
}
Why don't write something like
bool tabIndexZero = tabIndex == 0;
panelGeneral.Visible = tabIndexZero;
panelTolerances.Visible = !tabIndexZero;
btnProjSetNext.Enabled = tabIndexZero;
btnProjSetPrev.Enabled = !tabIndexZero;
This is twice as shorter! Benefit from knowledge of basic Boolean rules, which are, I believe, "must know" for every developer.
|
|
|
|
|
or why not...
bool tabIndexZero = tabIndex == 0;
panelGeneral.Visible = btnProjSetNext.Enabled = tabIndexZero;
panelTolerances.Visible = btnProjSetPrev.Enabled = !tabIndexZero;
or
panelTolerances.Visible = btnProjSetPrev.Enabled = !(panelGeneral.Visible = btnProjSetNext.Enabled = tabIndex == 0);
...anyway, the code you claim to be so bad is certainly much more readable, and also convenient if you need to change the logic (i.e. tabIndex = 0 should now set all values to true)
I am not saying it is good practice however
I may or may not be responsible for my own actions
modified on Wednesday, April 6, 2011 12:18 PM
|
|
|
|
|
musefan wrote: or why not...
Because that repeats the evaluation and a single answer is easier to update (invert as per your example) than a multitude of lines. It would also reduce the canche of introducing a bug because of a typo in the expression that's going to be evaluated.
Nitpicking again..
I are Troll
|
|
|
|
|
Eddy Vluggen wrote: Because that repeats the evaluation
good point
I may or may not be responsible for my own actions
|
|
|
|
|
musefan wrote: good point
Meh, too much VB, didn't notice the single equation-character
I are Troll
|
|
|
|