Andreas found this in a rather large, rather ugly production code base.
private static void LogView(object o)
{
try
{
ArrayList al = (ArrayList)o;
int pageId = (int)al[0];
int userId = (int)al[1];
// ... snipped: Executing a stored procedure that stores the values in the database
}
catch (Exception) { }
}
This function accepts an object
of any type, except no, it doesn't, it expect that object to be an ArrayList
. It then assumes the array list will then store values in a specific order. Note that they're not using a generic ArrayList
here, nor could they- it (potentially) needs to hold a mix of types.
What they've done here is replace a parameter list with an ArrayList
, giving up compile time type checking for surprising runtime exceptions. And why?
"Well," the culprit explained when Andreas asked about this, "the underlying database may change. And then the function would need to take different parameters. But that could break existing code, so this allows us to add parameters without ever having to change existing code."
"Have you heard of optional arguments?" Andreas asked.
"No, all of our arguments are required. We'll just default the ones that the caller doesn't supply."
And yes, this particular pattern shows up all through the code base. It's "more flexible this way."
