malawski / cloudworkflowsimulator

Other
24 stars 20 forks source link

Added runtime checks to ensure correctness. #107

Closed bryk closed 10 years ago

bryk commented 10 years ago

@Mequrel Review this and accept/reject.

I've (re)added runtime sanity checks. Why? Because it was much easier than fixing validator. Additionally, now I like the idea of runtime sanity checks to verify interactions.

Mequrel commented 10 years ago

I like "sanity checks" only in one case: when they are validating input contract (design by contract principle). I would suggest refactoring your code to achieve this. i.e. you have a method/service that terminates a job. the first line of it is to check if job is running now. Also I don't like throwing very general RuntimeException. It's better to use IllegalArguementException, IllegalStateException or NullPointerException.

Generally sanity checks are ok for me when they are only to detect programmer errors and this use case seems to be like that.

bryk commented 10 years ago

Updated. @Mequrel check now.

Mequrel commented 10 years ago

LGTM