nus-cs2113-AY2021S2 / forum

5 stars 0 forks source link

[Code Quality] Question about Exceptions #42

Closed Leeyp closed 3 years ago

Leeyp commented 3 years ago

I received this comment on my PR, but I don't quite understand what it means:

image

"It is good that new exception classes are created for different purpose. However, in this case, it currently seems to be doing the same thing as its parent class, Exception. Consider revising empty classes by removing them or updating them accordingly."

Aren't more exceptions to catch bad error cases better? That's what I understood from one of my previous questions.

okkhoy commented 3 years ago

I suspect you have empty class for the invalid command exception. When you create custom exception, it may be worthwhile to do something better than the generic exception. E.g., in the simplest case setting a custom exception message.

Leeyp commented 3 years ago

I have a custom exception message, but I put it into Ui class instead. So, it will print the message when this exception is triggered. Is that the wrong way?

Because otherwise my try/catch statement will be the one that is empty, and that is the one that the Coding Quality guidelines say is bad.

okkhoy commented 3 years ago

Think of overriding the getMessage()?

Leeyp commented 3 years ago

Great idea, thanks prof!