4

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?

5

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

| improve this answer | |
  • This definitely seems like a cleaner solution. I'll give it a try and come back with what I got. Thanks! – ilyketurdles Mar 28 '16 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 Mar 28 '16 at 20:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.