Was given this method
Watch that. Numb your brain to it. Allow it to consume you. Enjoy the logical impossibilities of else if (hardForecastList.Last().Id == hardForecast.Id || softForecastList.Count() == 0) inside of a nested foreach loop that iterates across softForecastList in the outer layer. Enjoy, as you quickly come to realize that the only difference between these blocks of code are that 0s are getting set in certain places.
When I asked about the logic inside of the code, the response:
"The logic is behind that method is to display a hard and soft forecast as a single record that has the same dd, site, os, cycle and memory type.
So basically, you have a single list containing elements that need to be combined based upon a specified predicate. Algorithmically similar to a merge sort. This was the approach taken.
Some people code like this. Cry.
I turned it into this:
Which I know is still wrong. There's a 1-2 line solution in LINQ using Joins and the such out there. Curious, I asked on StackOverflow, trying to simplify the problem domain to the real thing I don't understand. But I'm not sure I like those solutions as much, either.
upon further reflection I was able to improve it further by moving all the setter logic in the private method down into the ForecastByPercentDTO constructor. I was hesitant to do this, as this was an object not written by me and used in multiple places through the application.
ReplyDeleteOnce I actually looked at the object, I realized how ugly and badly written it actually was. Being down there already, I had to fix it. But now the code is a lot cleaner, and subtle bugs that wouldn't have been caught before are gone.