|
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
|
|
|
|
|
That's a brilliantly simple way to do things.
In short, there are two ways to solve a problem:
Just do it
Just do it, but do it in a better way!
-
Bits and Bytes Rules!
10(jk)
|
|
|
|
|
|
|
But, people like to use if statements!
I am happy if they at least use an else instead of another if. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
I think that both pieces of code are acceptable. While the original is much more verbose, that will lend itself to being understood by someone else. It additionally allows for easier maintenance should UI changes be needed.
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]
posting about Crystal Reports here is like discussing gay marriage on a catholic church’s website.[Nishant Sivakumar]
|
|
|
|
|
"Logics is really hard to learn?"
Don't you mean "Logics are really hard to learn?"
LOL, I'm just kidding.
Anyways, personally, I actually prefer the first one just because it's easier. My job involves 40 hrs a week of trying to decipher other people's written code and fix bugs. I love it, but I'll gladly take the simple-written code over the stuff I have to think over for a minute or so.
|
|
|
|
|
Even i like to go with first method, which makes the code more readable.
|
|
|
|
|
tabIndexZero is a poor choice for the identifier here, it should convey functionality; and statement reordering and better horizontal alignment could be used to improve readability, so maybe:
bool summaryMode = tabIndex == 0;
panelGeneral.Visible = summaryMode;
btnProjSetNext.Enabled = summaryMode;
panelTolerances.Visible = !summaryMode;
btnProjSetPrev.Enabled = !summaryMode;
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.
|
|
|
|
|
You beat me to it, I was about to say both examples are bad. I don't give a rats behind about how long the code it, but if it't doesn't tell me what it's doing then it's bad.
Also, the second example makes explicit the oppositeness of
PanelGeneral.Visible and PanelTolerances.visible
btnProjSetNext.Enabled and btnProjSetPrev.Enabled
That oppositeness might be coincidental rather than fundamental. If so the first code is actually better than the second.
-Richard
Hit any user to continue.
|
|
|
|
|
Richard A. Dalton wrote: You beat me to it
only just. 
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.
|
|
|
|
|
VUnreal wrote: This is twice as shorter!
What the heck does that mean? Twice as shorter than what? 
|
|
|
|
|
I just came across the main table in the SQL Server database that stores info for my company's website. It has 170,000 records... and no indexes. 
I have just remedied that by putting a clustered index on the primary key. I'll have to keep a look out for code that queries that table by fields other than the primary key... may need to add more indexes.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
