2

Our application is modulraized (Group of modules doing specific things). The modules have event handlers. These events could be fired from other modules or the application menu.

Situation:

Module A(which have UI) recieves an event "deleteitem". The event arguments should contain the item name to be deleted. But in this case it is null. Somewhere, somebody messed with something.

Question(s):

Should the Module throw? Rememeber, the module would be throwing inside an event handler and could crash the app as the module writer have no idea if the exception is handled.

The above scenario is a snaphot of a bigger question regarding throwing of excpetions from Modules which could result in application crashing. The argument against it is the application can continue working without the specific modules. Well then, who should ensure that - the module or the application?

Jimmy
  • 3,224
  • 5
  • 29
  • 47
  • 1
    If it doesen't throw what else would happen, do you want that? – Jodrell Apr 02 '12 at 10:48
  • If it doesnt throw, the application will continue. Do I want that?-that is the question. I dont want that as I know there is something wrong somewhere. The counter argument is let the application continue as the application can continue without the module performing the operation. – Jimmy Apr 02 '12 at 10:56

3 Answers3

5

If the item name is something that is expected to always be there and it being null is an exceptional circumstance, which should never happen, you should throw as now your application is in an unknown state that should never happen

If this is something that the caller can recover from, they will write their own exception handling routines to handle it.

Oded
  • 489,969
  • 99
  • 883
  • 1,009
  • This is what I also think. The answer of Maciej is the counter argument I am facing from my colleagues. – Jimmy Apr 02 '12 at 10:52
  • An actual exception, like this one, should be raised here. The exception can be logged and ignored further up the stack if appropriate. – Jodrell Apr 02 '12 at 12:38
  • @Jodrell - Well, if an attempt is made to access members of a passed `null` parameter, an exception _will_ be thrown. – Oded Apr 02 '12 at 12:39
  • @Oded, true, I agree. I mean to say, don't catch and suppress an exception, giving the caller the impression the function worked. You can log and suppress to prevent display to the user once at the top. http://stackoverflow.com/questions/1472498/wpf-global-exception-handler – Jodrell Apr 02 '12 at 12:47
  • @Oded. What do you think about an IApplicationErrorHandler interface is injected to the modules? The modules can call Reporterror(type) from the interface. This way the modules can report errors, but the interface implementation can decide whether the application should continue or not – Jimmy Apr 02 '12 at 16:28
  • @Jimmy - That's a way to handle _expected_ issues, not _unexpected_ ones and as such sounds OK. – Oded Apr 02 '12 at 19:04
3

Is it an essential event? Can you ignore it? I would treat that as an empty event and ignore it. I guess not being able to delete something is not mission critical. Good idea is to log this information as a warning or error perhaps, so you know it happened.

My rule would be that if something can be re-issued it is not essential but if it can't - it is important. But that can change depending on particular application context.

Bad things happen and if you can you should try to recover from them. My priority would be to ensure application (module) does not crash even if is fed wrong data but definitely make sure you know bad things happen (logging). Exception is the last resort if you're helpless and can't do anything else.

What I described here would best apply to separated modules like WCF services for example. If it is all in one app (modules are classes), probably best would be to return an exception but make sure caller can handle it. So modules separation is important here - the more separation the more error resiliency should there be. WCF services or Windows services should not crash.

Maciej
  • 7,871
  • 1
  • 31
  • 36
  • This is exactly the agument from my colleagues. So, is it ok discarding error state of the application asking your modules to do things? – Jimmy Apr 02 '12 at 10:54
  • Just added more to my answer. Bad things happen and if you can you should recover from them. Exception is the last resort if you're helpless and can't do anything else. – Maciej Apr 02 '12 at 10:57
  • I would strongly recommend against ignoring the error. As Oded mentions in his answer, the application is now in an unknown state. You have no control of what further execution will lead to. If you're lucky you'll end up with another exception which will be very difficult to know the root cause of. If you're not so lucky you'll end up with corrupt user data. – Torbjörn Kalin Apr 02 '12 at 11:01
  • @MaciejDopieralski So, if my application can continue without anybody complaing about an error situation and many more in the future the application is getting more and more instable as time goes by. Is that ok? – Jimmy Apr 02 '12 at 11:05
  • Again, depends on importance of event. In either case do not ignore it. Make sure logs get through to let know about issue. But crashing UI is the last resort in my view and it should happen if something unrecoverable happened (like not config or disk error etc.) – Maciej Apr 02 '12 at 11:11
  • Only if your modules are very tightly coupled - like classes in one app domain. If the are separate, independent modules go for non-exception approach. – Maciej Apr 02 '12 at 11:15
  • @Maciej Dopieralski. I thought the modules represent seperation of concerns. Is the module concerned about how the thrown exception is handled? – Jimmy Apr 02 '12 at 11:15
  • Right I have seen the comment. By "modularizing" I am targetting loose coupling(clearly). – Jimmy Apr 02 '12 at 11:18
  • To some "module" can mean just another assembly while to others - separate service. Just clarifying my answer. – Maciej Apr 02 '12 at 11:21
  • @MaciejDopieralski. I am not talking about WCF/Windows Services. I am talking about a WPF module which has a UI. – Jimmy Apr 02 '12 at 11:22
1

Do throw. React to any such exceptions by fixing the module that is throwing the malformed event, as soon as they are discovered.

The answer partly depends on how you build and version the product (whether the module that fires the event is maintained and built together with all the listeners).

Because you are describing a pretty extreme behavior, indicating a product defect, it is good to find it as early as possible. If you do not have a better method of alerting anyone who is using or testing the defective software, do throw.

On the other hand, do not throw in less extreme circumstances, such as when the event might be well formed, or when your own module might be responsible for it.

Product defects are cheapest to fix if found relatively early; but excessive throwing may cause defect propagation to unrelated modules (that will receive some but not all related events) and thus delay pinning the defect down.

Jirka Hanika
  • 13,301
  • 3
  • 46
  • 75
  • I think the same. I got an event in the module which is asking me to do something I cannot do and which clearly indicates an error somewhere in the application.I would think it is better to know this early rather then continuing with a instable application. – Jimmy Apr 02 '12 at 11:03