I'm having trouble cleaning up my code, and was wondering if there was any sort of pattern that I'm not aware of that can help me in this situation.
I need to process a list of objects in various methods, somewhere in those methods, an object in the list can fail for a specific reason, and the reasons can vary. I need to output the objects that failed (they can be converted to strings without issue) and more importantly, WHY they failed.
Due to the large number of objects I process (couple hundred to a couple thousand), I can't individually list all the ones which had problems.
Before I had just one reason something failed, like "Invalid ID". So I created a list that was "FailedAccounts", and then output something like "The following accounts had an invalid ID:" Then output all the elements in the list with a comma in between them. That worked fine.
Then I was told "Well, these ones can fail due to another reason". So I created a second list, call it "XFailedAccounts". So now I have
"The following accounts have an invalid ID:"
"The following elements have X":
Then I was given another criteria. And then another, and then another.
4+ lists is just plain messy. I need a way to organize the items and then group them together appropriately.
I considered a dictionary/hash with the reason as the value and the object as the key, but then I'd have to search and group by value, which defeats the purpose of a hash, I think, but still provides better organization than lists.
Attempt 1
Then I explored the possibility of creating different "list" types that inherit from List, and just add a "reason" field. The accounts are then added to the specific list. So I would have something like:
class FailedAccountsList : List<string>
{
string reason = "Failed Account ID"
}
public static FailedAccounts
{
FailedAccountsList reason1 = new FailedAccountsList();
FailedAccountsList reason2 = new FailedAccountsList();
}
Then I'd hold all these different lists as static objects in a static class called "Warning Lists". And populate the individual lists as I go.
Sure, this is more structured than having 4+ individual lists, but seems really messy and ugly.
Attempt 2
I made one SINGLE list, and created an object of type "FailedAccount" that holds an account and a reason. After I'm done processing all the objects, I search for distinct reasons, and then pull them out and place them into lists.
class FailedObj
{
Object objectThatFailed;
string Reason; (I can make this into an actual type instead of a string)
}
The problem here is that I have to iterate over the "full" list N * distinct_reasons times. Which I guess is just N times, and then again for each item in the list to output the individual objects. Not terrible, not great either. Realistically with a maximum of maybe 4000 objects it's not a huge performance hit. Even taking into account the overhead for 4000 objects, it's not big in the grand scheme of things. But I'm still not satisfied.
How do I find the best way to do things like this? Do I need to sweat over optimization if big O is not quadratic? How do I get better and designing cleaner code?
1 Answer 1
Why not something like:
var failedAccountsByReason = new Dictionary<Reason, List<FailedAccount>>();
You can iterate by the dictionary Keys
to look all the reasons, check the individual accounts easily, as well as counting the number of accounts that failed for any reason
-
This definitely seems like a cleaner solution. I'll give it a try and come back with what I got. Thanks!ilyketurdles– ilyketurdles2016年03月28日 18:46:15 +00:00Commented Mar 28, 2016 at 18:46
-
So I went with your suggestion, and made a few changes. This way made it way cleaner and doesn't require reflection to know the reasons why an account was failed. Simple and great. Don't know why I didn't think of that when I was considering a dictionary. Thank you.ilyketurdles– ilyketurdles2016年03月28日 20:19:03 +00:00Commented Mar 28, 2016 at 20:19