So, a colleague inherited a project to restructure a product..
-
He doesn't work here anymore (actually can be anyone of 4 people, but none of them work here now). The thing is that all of them bragged about how their code was better and how they were the best I don't want to see the worst. X|
-
He called for my help when he couldn't understand a piece of code. After 2 hours digging, we finally understood what the code does. A convoluted piece of junk that just shows a string on a field on the page. But the most facepalming thing we found was this piece of "code":
public string GetLimitMessage(int total, int limit){
if(total > limit)
try
{
return "The Limit has been reached!"
}
catch(Exception)
{
return "";
}
else
return string.Empty;
}Aside from translation (the original is on Portuguese), that's the exactly code we found. We are still facepalming.
-
So I assume all over the codebae you have something like this?
if (GetLimitMessage(a, b).Length > 0)
MessageBox.Show(GetLimitMessaage(a, b));The good thing about pessimism is, that you are always either right or pleasently surprised.
Are you mad! That would be far too risky to write it like that, far better would be:
if (!GetLimitMessage(a, b).equals(GetLimitMessage(1, -1)) {
if (!String.IsNullOrEmpty(GetLimitMessage(a, b)) {
MessageBox.Show(GetLimitMessaage(a, b));
}
else
{
MessageBox.Show("The monkey's dead!");
}
}speramus in juniperus
-
So I assume all over the codebae you have something like this?
if (GetLimitMessage(a, b).Length > 0)
MessageBox.Show(GetLimitMessaage(a, b));The good thing about pessimism is, that you are always either right or pleasently surprised.
It's an APS.NET MVC 3 Application, so it's: Controller:
if(Helper.GetLimitMessage(total, 100)) //Yep, 100 is harcoded on every line it's used... { ViewBag.LimitMessage = Helper.GetLimitMessage(total, 100); ViewData\["LimitMessage"\] = Helper.GetLimitMessage(total, 100); }View:
@if(ViewBag.LimitMessage != "") { @Helper.GetLimitMessage(total, 100) } else if (ViewData\["LimitMessage"\] != null) { @ViewData\["LimitMessage"\] }Even my coworker who normally doesn't rant on code is cursing this one.
-
Are you mad! That would be far too risky to write it like that, far better would be:
if (!GetLimitMessage(a, b).equals(GetLimitMessage(1, -1)) {
if (!String.IsNullOrEmpty(GetLimitMessage(a, b)) {
MessageBox.Show(GetLimitMessaage(a, b));
}
else
{
MessageBox.Show("The monkey's dead!");
}
}speramus in juniperus
-
Are you mad! That would be far too risky to write it like that, far better would be:
if (!GetLimitMessage(a, b).equals(GetLimitMessage(1, -1)) {
if (!String.IsNullOrEmpty(GetLimitMessage(a, b)) {
MessageBox.Show(GetLimitMessaage(a, b));
}
else
{
MessageBox.Show("The monkey's dead!");
}
}speramus in juniperus
Surely a MessageBox is rather "old hat"? How do you display a Fail Whale in a MessageBox?
-
It's an APS.NET MVC 3 Application, so it's: Controller:
if(Helper.GetLimitMessage(total, 100)) //Yep, 100 is harcoded on every line it's used... { ViewBag.LimitMessage = Helper.GetLimitMessage(total, 100); ViewData\["LimitMessage"\] = Helper.GetLimitMessage(total, 100); }View:
@if(ViewBag.LimitMessage != "") { @Helper.GetLimitMessage(total, 100) } else if (ViewData\["LimitMessage"\] != null) { @ViewData\["LimitMessage"\] }Even my coworker who normally doesn't rant on code is cursing this one.
-
Are you mad! That would be far too risky to write it like that, far better would be:
if (!GetLimitMessage(a, b).equals(GetLimitMessage(1, -1)) {
if (!String.IsNullOrEmpty(GetLimitMessage(a, b)) {
MessageBox.Show(GetLimitMessaage(a, b));
}
else
{
MessageBox.Show("The monkey's dead!");
}
}speramus in juniperus
Woah up there, you're missing a major source of defensive coding here...
try {
if (GetLimitMessage(a,b) != null && GetLimitMessage(a,b).Equals(GetLimitMessage(1, -1))) {
if (!string.IsNullOrEmpty(GetLimitMessage(a,b)) {
try {
MessageBox.Show(GetLimitMessage(a,b));
}
catch (Exception ex) {
throw;
}
}
}
}
catch (Exception ex) {
MessageBox.Show("The monkey's dead!");
}"If you don't fail at least 90 percent of the time, you're not aiming high enough." Alan Kay.
-
It's an APS.NET MVC 3 Application, so it's: Controller:
if(Helper.GetLimitMessage(total, 100)) //Yep, 100 is harcoded on every line it's used... { ViewBag.LimitMessage = Helper.GetLimitMessage(total, 100); ViewData\["LimitMessage"\] = Helper.GetLimitMessage(total, 100); }View:
@if(ViewBag.LimitMessage != "") { @Helper.GetLimitMessage(total, 100) } else if (ViewData\["LimitMessage"\] != null) { @ViewData\["LimitMessage"\] }Even my coworker who normally doesn't rant on code is cursing this one.
Is this the real life? Is this just fantasy? caught in a landslide no escape from reality open your eye's look up to the ceiling and stare I'm just a poor coder, I get no sympathy because I make simple things, make them work respect my deadlines, don't be a jerk Any way management blows, doesn't really matter to meeeee To meeeeeee MAMAAAAA I just killed my coworker put a gun against his head pulled my trigger, now he's dead... :thumbsup:
.
-
It's an APS.NET MVC 3 Application, so it's: Controller:
if(Helper.GetLimitMessage(total, 100)) //Yep, 100 is harcoded on every line it's used... { ViewBag.LimitMessage = Helper.GetLimitMessage(total, 100); ViewData\["LimitMessage"\] = Helper.GetLimitMessage(total, 100); }View:
@if(ViewBag.LimitMessage != "") { @Helper.GetLimitMessage(total, 100) } else if (ViewData\["LimitMessage"\] != null) { @ViewData\["LimitMessage"\] }Even my coworker who normally doesn't rant on code is cursing this one.
:doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh: :doh:
Getting information off the Internet is like taking a drink from a fire hydrant. - Mitchell Kapor
