That's what you get when you don't perform code reviews
-
A former employee of ours left some really wonderful smells in our code, for example a class with no less than 8 constructor parameters. And right now I stumbled across a method named
TryGetUsergroupByNameOrTostringDss(). Thanks BH, you know who you are... I guess it's my fault as well - now I can see how important code reviews for every code would have been. But it's too late to mourn now and I have to clean out the manure on my own :(Regards, mav -- Black holes are the places where God divided by 0...
-
A former employee of ours left some really wonderful smells in our code, for example a class with no less than 8 constructor parameters. And right now I stumbled across a method named
TryGetUsergroupByNameOrTostringDss(). Thanks BH, you know who you are... I guess it's my fault as well - now I can see how important code reviews for every code would have been. But it's too late to mourn now and I have to clean out the manure on my own :(Regards, mav -- Black holes are the places where God divided by 0...
There's nothing necessarily wrong with eight parameters for a constructor.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013 -
There's nothing necessarily wrong with eight parameters for a constructor.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013 -
There's nothing necessarily wrong with eight parameters for a constructor.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013It's the only constructor and none of the parameters are optional... Oh, and did I mention that the whole class has exactly 0 comments?
Regards, mav -- Black holes are the places where God divided by 0...
-
Like a Tuple<,,,,,,,> ? :laugh:
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
-
There's nothing necessarily wrong with eight parameters for a constructor.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013 -
Like a Tuple<,,,,,,,> ? :laugh:
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
I'm no longer allowed to pass a Tuple-parameter after my last review :-\ So I now create a specific object:
class MyList: List<Tuple<int, string, string>> { }
-- Joking aside, the
[ProcessStartInfo](https://msdn.microsoft.com/nl-nl/library/system.diagnostics.processstartinfo%28v=vs.110%29.aspx)[[^](https://msdn.microsoft.com/nl-nl/library/system.diagnostics.processstartinfo%28v=vs.110%29.aspx "New Window")]class would be a nice example of limiting the amount of parameters required, while still keeping it readable.Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)
-
I'm no longer allowed to pass a Tuple-parameter after my last review :-\ So I now create a specific object:
class MyList: List<Tuple<int, string, string>> { }
-- Joking aside, the
[ProcessStartInfo](https://msdn.microsoft.com/nl-nl/library/system.diagnostics.processstartinfo%28v=vs.110%29.aspx)[[^](https://msdn.microsoft.com/nl-nl/library/system.diagnostics.processstartinfo%28v=vs.110%29.aspx "New Window")]class would be a nice example of limiting the amount of parameters required, while still keeping it readable.Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)
:laugh:
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
-
A former employee of ours left some really wonderful smells in our code, for example a class with no less than 8 constructor parameters. And right now I stumbled across a method named
TryGetUsergroupByNameOrTostringDss(). Thanks BH, you know who you are... I guess it's my fault as well - now I can see how important code reviews for every code would have been. But it's too late to mourn now and I have to clean out the manure on my own :(Regards, mav -- Black holes are the places where God divided by 0...
-
I really think it depends on the requirements of the object and how it fits into the existing code's style.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013 -
It's the only constructor and none of the parameters are optional... Oh, and did I mention that the whole class has exactly 0 comments?
Regards, mav -- Black holes are the places where God divided by 0...
I've got a class with 12 constructor parameters but the class represents a single row of data from a database so it is a discrete object. I also did full xml comments on everything so I don't forget why I wrote it. Would that be flagged in a code review?
if (Object.DividedByZero == true) { Universe.Implode(); } Meus ratio ex fortis machina. Simplicitatis de formae ac munus. -Foothill, 2016
-
I really think it depends on the requirements of the object and how it fits into the existing code's style.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013John Simmons / outlaw programmer wrote:
I really think it depends on the requirements of the object and how it fits into the existing code's style.
I think it depends on the amount of switches. Easy example in the ProcessStartInfo[^] class. It does not depend on how it fits into the "style". Show me a more readable version of the same using a constructor and a param for each switch, then I'll accept it may depend on requirements.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)
-
John Simmons / outlaw programmer wrote:
I really think it depends on the requirements of the object and how it fits into the existing code's style.
I think it depends on the amount of switches. Easy example in the ProcessStartInfo[^] class. It does not depend on how it fits into the "style". Show me a more readable version of the same using a constructor and a param for each switch, then I'll accept it may depend on requirements.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)
Matching the current coding style goes a long way towards "readability". Besides, I'm not here to argue semantics. I merely stated that there's nothing necessarily wrong with a constructor with eight parameters. There are often many more considerations than "I think this code sucks". I'm not really interested enough to list all the ones I can think of right off the top of my head.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013 -
unless the OP means 8 overloaded constructors with different parameter signatures? (in which case defaults could definitely help)
He never said anything about overloads, so I think it's safe to assume there is one constructor with eight parameters.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013 -
I've got a class with 12 constructor parameters but the class represents a single row of data from a database so it is a discrete object. I also did full xml comments on everything so I don't forget why I wrote it. Would that be flagged in a code review?
if (Object.DividedByZero == true) { Universe.Implode(); } Meus ratio ex fortis machina. Simplicitatis de formae ac munus. -Foothill, 2016
Foothill wrote:
I've got a class with 12 constructor parameters but the class represents a single row of data from a database
I don't do that with column data anymore unless I need the object to be initialized a certain way. I just use object initializers.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013 -
Matching the current coding style goes a long way towards "readability". Besides, I'm not here to argue semantics. I merely stated that there's nothing necessarily wrong with a constructor with eight parameters. There are often many more considerations than "I think this code sucks". I'm not really interested enough to list all the ones I can think of right off the top of my head.
".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013John Simmons / outlaw programmer wrote:
Matching the current coding style goes a long way towards "readability".
So you'd prefer to implement that same class as a constructor with 16 switches? Show me a "current coding style" where that is more readable than a single class as described on MSDN :)
John Simmons / outlaw programmer wrote:
I'm not really interested enough to list all the ones I can think of right off the top of my head.
I did not ask you to.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)
-
-
A former employee of ours left some really wonderful smells in our code, for example a class with no less than 8 constructor parameters. And right now I stumbled across a method named
TryGetUsergroupByNameOrTostringDss(). Thanks BH, you know who you are... I guess it's my fault as well - now I can see how important code reviews for every code would have been. But it's too late to mourn now and I have to clean out the manure on my own :(Regards, mav -- Black holes are the places where God divided by 0...
-
Yes! And you can try to provide alternative constructors with fewer parameters. Do you really always need 8 parameters?
Get me coffee and no one gets hurt!
Taking the same ProcessStartInfo class as an example. Yes, I need those 16 switches, and no, it would not become more readable if you provided an overloaded version of each possible combination.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)
-
It's the only constructor and none of the parameters are optional... Oh, and did I mention that the whole class has exactly 0 comments?
Regards, mav -- Black holes are the places where God divided by 0...
mav.northwind wrote:
the whole class has exactly 0 comments
That's exactly how much comments code should have! :D Great code requires no comments. Most comments are bad anyway. I'm so allergic to comments I even wrote a tip on why you shouldn't comment (and the rare case you should): Write comments that matter[^]
Visit my blog at Sander's bits - Writing the code you need. Or read my articles at my CodeProject profile.
Simplicity is prerequisite for reliability. — Edsger W. Dijkstra
Regards, Sander
