Bonus points to anyone that can figure this one out.

Andy Brummer

So, what is this code doing and why? And I'm cleaning up hundreds of these methods. :^) :sigh: :doh:

    public static ArrayList GetAll(CDataAccess db)
    {
        ArrayList dataObjects = new ArrayList();
        ArrayList rtn = new ArrayList();
        try
        {
            dataObjects = Data.ExchangeRate.GetAll(db);
            IEnumerator ie = dataObjects.GetEnumerator();
            while (ie.MoveNext())
            {
                using (ExchangeRate businessExchangeRate = new ExchangeRate())
                {
                    using (Data.ExchangeRate currentData = ie.Current as Data.ExchangeRate)
                    {
                        businessExchangeRate.\_dataAccess = currentData;
                        rtn.Add(businessExchangeRate);
                    }
                }
            }
        }
        catch (Exception ex)
        {
            throw new Exception("An error occurred while calling the GetAll() method.", ex);
        }
        finally
        {
        }
        return rtn;
    }

I can imagine the sinking feeling one would have after ordering my book, only to find a laughably ridiculous theory with demented logic once the book arrives - Mark McCutcheon

PIEBALDconsult

It seems the author doesn't know foreach. Nor how to cast.

Andy Brummer

That's just the start, it gets better. Check out the using statements and where the objects are going.

I can imagine the sinking feeling one would have after ordering my book, only to find a laughably ridiculous theory with demented logic once the book arrives - Mark McCutcheon

Graeme_Grant

People like to make things hard for themselves. What was this guy smoking when he was coding? My thoughts are that he's an ex-C++ programming who landed a C# project. Looks like this is part of the BO layer that works with multiple databases (db) that contains exchange rates. The database pointer is passed down to the data access layer, data is retrieved and parsed into a simple list and returned to the caller. Not exactly an efficient way of doing it... Am I close? ;P

Andy Brummer

You're close. It's very similar to a common error new C++ developers make, and is something that garbage collection is supposed to fix, but they've managed to do it in C#. Think about the lifetime of the objects created in the loop and the using statement.

I can imagine the sinking feeling one would have after ordering my book, only to find a laughably ridiculous theory with demented logic once the book arrives - Mark McCutcheon

Graeme_Grant

Yeah... Trying to return data that's already expired! Nice one... lol ;P

Andy Brummer

Yeah, since the dispose method closes database connections, then just wrap the object in a using and it will be automatically closed. :omg: They cut and pasted a standard dispose method including the check that it only runs once, so you can only close the database connection once, which is only an issue when you save one of the objects. :doh:

I can imagine the sinking feeling one would have after ordering my book, only to find a laughably ridiculous theory with demented logic once the book arrives - Mark McCutcheon

bigbrownbeaver

I don't understand .Net memory management myself... I seriously think a delete operator wouldnt hurt. Why is there a new and no delete? I don't blame this guy for making mistakes, at least his code is well formatted. I work on VB code that looks like it was written by monkies in a scientific experiment.

Andy Brummer

so,

for(int i=0; i<source.length;> {
result[i] = source[i];
source[i].~object();
result[i].~object();
}
return result;

Makes sense to you?

I can imagine the sinking feeling one would have after ordering my book, only to find a laughably ridiculous theory with demented logic once the book arrives - Mark McCutcheon