Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a list of objects that need to be processed in some way. That way may not be correct for an item and thus raise an exception. In pseudo-code, it would look like:

proccessItems (list)
  error = new empty list
  for item in list:  
    try:
      process_item(item)
    catch Whatever, err:
      error.append(err)
  return error

Is there a pattern for doing this sort of things? Are there any anti-patterns to avoid?

A better code (exception not used for control flow), but still following the same logic would be:

proccessItems (list)
  error = new empty list
  for item in list:  
    ret = process_item(item)
    if null != ret 
      error.append(ret)
  return error
share|improve this question
    
guess thats just mistake, but you create the error list inside loop, so you would return only error from last item –  user470365 Nov 25 '11 at 10:22
    
@user470365: Yes, that is indeed a bug. Code edited now. Many thanks. –  Sardathrion Nov 25 '11 at 10:30
    
This depends on the level of abstraction, somewhat. If this is low-level, throw the exception and stop processing. If this is higher level, attempt to only process things that should be valid. Your validity checking code shouldn't throw exceptions, it should return some object that indicates valid/invalid/invalid-and-here's-the-error (boolean v. null with some specific object). You should probably write a roughly equivalent method anyways, that you call when you inside of process_item(). –  Clockwork-Muse Nov 25 '11 at 22:36
add comment

2 Answers 2

up vote 4 down vote accepted

First of all, you shouldn't use exceptions for normal logic flow.

You should ask yourself why you are trying to iterate over items which you can't process in the first place. Probably designing the code in such a way you only have items in your list which you can process is a better idea.

In case you do feel like all those items belong in that one list. Instead of throwing an exception you should still opt to use a conditional check (or polymorphism) to see whether the item is suitable for processing.

I can't specify 'official' anti-patterns but I would consider the following:

  • Using exceptions for logic flow.
  • Using type checking or other conditional checks instead of proper design/polymorphism.
share|improve this answer
    
I get the list from an external source I have no control over so cannot know that it will be correct. Pre-processing would look exactly the same since I need to return a list of errors to the external source. So, instead of process_item(), you would have a validate_item(). Type are not necessary enough. I could process names of countries (for example) so type checking would be of limited use. –  Sardathrion Nov 25 '11 at 10:54
    
@Sardathrion: If your intent is to catch and report errors, of course that is just what you should do. Depending on the scenario either catching exceptions is valid, and you could look into existing logging techniques, or you should replace the exception with validation checks. –  Steven Jeuris Nov 25 '11 at 10:59
    
Other than (at a high level) specifically catching and reporting errors/exceptions, do not return some sort of error code; there's too much of a chance that someone will just ignore it, or that they may interpret it wrong. Or, heaven forbid, the permitted returned value changes, meaning a previous 'error' is now 'good'. Make your code as simple and obvious to use as possible - this includes forcing the client to code for exceptions, even if they explicitly ignore them (emprty catch block. –  Clockwork-Muse Nov 25 '11 at 22:07
add comment

I would pass the error list to the process_item function as well, then you'd have a simple iterator and you wouldn't need to throw exceptions to be caught by the proccessItems function.

That way maybe you could replace the exceptions for simple error messages ( like a string ) and avoid using them for control flow since that is not why they were designed in the first place. ( using them like this is a bad practice )

Keep in mind that exceptions ( depending on the language ) are expensive operations so given a large enough list the current example could be a problem.

share|improve this answer
    
Using string literals for control flow is also a bad practice. ;p –  Steven Jeuris Nov 25 '11 at 11:45
    
@StevenJeuris :/ they are better than exceptions and Sardathrion can change them for whatever error type makes more sense for the application. –  Thanos Papathanasiou Nov 25 '11 at 11:53
    
Actually, they're potentially even worse. For one thing, it's not necessarily internationalization safe. For another, client code becomes fairly brittle - do you check for a 'success' message, or a 'failure' message, and what do you do if you get something else? With a decent architecture, certain sets of flow control can either be removed, or restricted to lower levels of abstraction. –  Clockwork-Muse Nov 25 '11 at 22:27
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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