search

ckeditor / ckeditor4-java-core

The development repository of CKEditor 4 for Java (core).
Other
5 stars 9 forks source link

We need to add some logging for caught exceptions. #3

Closed jswiderski closed 9 years ago

jswiderski commented 9 years ago

Classes: CKEditorconfig, CKEditorInsertTag and CKEditorTag all throw exceptions. We should add a logger there.

jswiderski commented 9 years ago

Fixed or partially Fixed with 9c175c0.

We are no longer catching Exception (what is a bad practice). We are no longer throwing exceptions from finally block because they could swallow exceptions thrown in catch block (another bad practice). Instead we are logging exceptions that we catch. I'm not sure how previous code got there in the first place.

I'm still thinking about solution for CKEditorconfig.

jswiderski commented 9 years ago

Furtner fixes with 58512b8

I have made some research and I really liked the solution mentioned here http://stackoverflow.com/a/1274822. Returning empty configuration object instead of null as it avoids further ifs.

About catching exception and returning null - it is debatable whether this is a bad practice or not. It depends on particular use case however in this one returning empty configuration object makes more sense.

Another link: https://today.java.net/article/2006/04/04/exception-handling-antipatterns