Programmatic Malpractice

William — Thu, 01 Feb 2007 03:23:00 GMT

In the category of "I couldn't make something like this up even if I tried" comes the following code. Rather than try to dissect what it is that's wrong with it, it's a lot easier to find what's right with it. If the person that wrote this cashes their paycheck, they should be charged with theft. We have all been in a hurry and written code that we aren't proud of. We all are guilty of oversights. But this was done by a self proclaimed guru and expert, and when you read the comments that were attached to it, it's clear that this was done intentionally. So I'm opening the floor to pointing out what's wrong with it, or for my really advanced readers, what's right with it (God knows a lot can go wrong calling the AddNew method of a DataTable and wrapping such a dangerous operation in catch block catching System.Exception is absolutely critical): Read the code first, then read the comments associated with it. P.S. I'm warning you up front that I'm not responsible if you start barfing, become overwhelmed by the ugliness of it and gouge your eyes out as solace or if your brains succumbs to spontaneous combustion. Yep, a 'professional consultant' wrote this work of art. As a consultant, I guess I should find comfort in stuff like this b/c as long as there are people like this committing malpractice, I mean working, then I'll always have a job. On the other hand, this sort of stuff is so bad and so inexcusably terrible , that my heart goes out to this person's victim, urr, I mean client.

public static decimal EvalNumber(string strExpr, ref bool
calcSuccessful, ref string NegativeDelim)
{
calcSuccessful=false;
NegativeDelim = "-";
DataTable dt = new DataTable();
try
{
dt.Columns.Add("Expr", typeof(Decimal), strExpr);
}
catch
{
// tough decision they entered a error
// colud be as simple as text/words
// could be division by zero
return (0);
}

try
{
DataRow dr; dr = dt.NewRow();
dt.Rows.Add(dr);
}
catch
{
return (0);
}

decimal TempReturn=0;
try
{
TempReturn=Convert.ToDecimal(dt.Rows[0]["Expr"]);
calcSuccessful=true;
}
catch
{
calcSuccessful=false;
}

// expressions like (9) will resolve as positive 9 which is wrong
// expressions like (9+2*12)*20 will obviously be a real formula
if (calcSuccessful == true && strExpr.IndexOf("(") == 0)
{
string tempStr = strExpr;
tempStr = tempStr.Replace("(", "");
tempStr = tempStr.Replace(")", "");
// parentheses are stripped

decimal tempNum = 0;
try
{
tempNum = Convert.ToDecimal(tempStr);
// (9) == 9 so we must turn it to -9
if (tempNum == TempReturn)
{
TempReturn = TempReturn * -1;
NegativeDelim = "(";
}
}
catch
{
// nothing to do, all is well
}
} // end if parentheses issues

return (TempReturn);

} // end EvalNumber

private void ITDataGridView_CellParsing(object s,
DataGridViewCellParsingEventArgs e)
{
string tempMsg = "ITDataGridViewFund_CellParsing";
Logger.DGVStatusGrid(tempMsg, s, e, this);

if (CellTypeCalc(e.RowIndex, e.ColumnIndex, this) == false)
return;

// we are in a row that allows formulas
bool evalSucceeded = false;
string negativeDelim = "";
decimal decMoney = 0;
string CellValue =
this.Rows[e.RowIndex].Cells[e.ColumnIndex].EditedFormattedValue.ToString();
decMoney = Utils.EvalNumber(CellValue, ref evalSucceeded,
ref negativeDelim);
if (evalSucceeded)
{

// set format based on ( or -
if (negativeDelim == "(")

this.Rows[e.RowIndex].Cells[e.ColumnIndex].Style.Format =
"#,##0;(#,##0)"; ;
if (negativeDelim == "-")

this.Rows[e.RowIndex].Cells[e.ColumnIndex].Style.Format =
"#,##0;-#,##0";
}

} // end ITDataGridView_CellParsing

-----------------------------

Quote from the Guru that wrote this:

I tried every event to set .Value to the result of that formula. I
tried dozens of desperate acts including just trying to SendKeys the
Calc result to the cell (even that hack is impossible to get to work).
Despite printing dozens of articles from Google and reading and
empirically experimenting, days of frustrating no answer were my fate.
But its ok, they had not resolved this in months and were going to say
"no" to that user requirement before they handed it to me. They had
made a pretty good app, otherwise that is a 1:1 knockoff functionality
wise of another .net app written in 1 C# file with the Infragistics
grid and spaghetti code everywhere, extraneous code, ad-hoc SQL, etc.
whereas they wrote a fresh code base in 2 dozen sanely organized files
and using Sprocs instead. They just needed a few bugs and enhancements
fixed, and the code refactored in places for the new code and they
were quite burn out on taming the DataGridView with very little,
incomplete and confusing and contradictory docs whereas I am not burnt
out.

The two things that maybe could help but the docs/Google searches were
as clear as mud i.e.
e.FormattingApplied=true
e.ParsingApplied=true
seemed to not work well. They are normally false but if set to True
they tended to create recursive DGV events in an infinite loop and
overflow the stack. Ouch!

Finally the answer turns out to be to use
_CellParsing

and VERY IMPORTANT use these 2 lines:

e.Value = newValue;
e.ParsingApplied = true;

and never do:
this.Rows[e.RowIndex].Cells[e.ColumnIndex].Value = newValue;
e.ParsingApplied = true;

even though its basically the same thing because DataError will fire
and recursion will occur etc.

The users had 2 needs that sound simple but are quite tricky...

1) a cell that understands a formula
2) that cell if you enter -33 it should format as -33
if you enter (33) it should format as (33)
(ouch harder than it sounds)

and the Eval function I use i.e. is crude and convoluted because of
requirement 2 and the fact that when it works as designed (33)
resolves to +33 if one does not compensate since it considers
parentheses to be an algebraic operator i.e. to make (2+3)*9 resolve
without the multiplication occurring first via the order of operation
rules. I probably will redo the EvalNumber function soon to ve cleaner
and maybe with generics to support more numeric types than decimal
(int, double, etc.) but this app only needs decimal.

UPDATE: You've heard the adage 'the customer is always right' haven't you? Well, I guess the developer that wrote this has his own version "If the customer doesn't complain, then the code doesn't suck" Hysterically, since the customer was ok with the end result, it's not bad code. I guess you couldn't write code that met the customer's needs that wasn't so lame? The fact is, that a lot of code problems don't show themselves at first. It's only after stuff doesn't scale or port well, that people realize it's crap. Trust me dude, you could have written better code that they would have been ok with and rationalize it however you like, people bragging about being gurus shouldn't start writing stuff like this.

/bill's House O Insomnia : WTF

Programmatic Malpractice