Code Suggestions

[Jimmacle]

These are some things I noticed about the current code that I think could be improved at first glance. If you agree with any of them I could submit pull requests with the relevant code changes.

Exceptions for flow control

https://github.com/lifebeyondfife/Decider/blob/5c2d8af70adab86e0edb509590849320bb7302f4/Csp/Integer/StateInteger.cs#L136-L149 Using exceptions and try/catch blocks for flow control within StateInteger obscures the program flow and introduces significant performance overhead compared to other strategies. Conveniently, it seems like every searching method that throws a DeciderException returns void. This means all these methods can relatively easily be rewritten to return a boolean indicating whether the search should terminate or not rather than throwing an exception. I would combine this with rewriting the while loops to use the condition that currently throws a DeciderException as the exit condition rather than looping infinitely to improve readability. In general, throwing exceptions should be reserved for when the code is put in an unexpected (exceptional, if you will) state that can't be recovered from.

Explicit interface implementation

https://github.com/lifebeyondfife/Decider/blob/5c2d8af70adab86e0edb509590849320bb7302f4/Csp/Integer/StateInteger.cs#L35 Types that implement interfaces should do so implicitly rather than explicitly. This allows the interface implementation to be accessed without requiring an explicit cast to the interface type. In my experience, explicit interface implementation is only really needed when implementing two or more interfaces that define conflicting member signatures.

Timing

https://github.com/lifebeyondfife/Decider/blob/5c2d8af70adab86e0edb509590849320bb7302f4/Csp/Integer/StateInteger.cs#L193-L194 For performance-related timing, a System.Diagnostics.Stopwatch should be used instead of DateTime because it's more precise and won't be affected by changes to the system clock. For example, the current solver will time out after an incorrect amount of time if the system clock is changed after the solver has been started.

foreach/LINQ usage in performance-critical loops

https://github.com/lifebeyondfife/Decider/blob/5c2d8af70adab86e0edb509590849320bb7302f4/Csp/Integer/StateInteger.cs#L213-L215 LINQ tends to make hidden allocations that increase pressure on the garbage collector and can have a bigger impact on performance than a non-LINQ equivalent implementation so it should be avoided if high performance is the goal. For example, calling Where in the above code allocates an iterator object behind the scenes. foreach can have similar behavior when used directly with a collection type.

[lifebeyondfife]

Hi @Jimmacle.

Thank you so much for the the detailed inspection of the Decider codebase, and the equally descriptive write-up and suggestions. I've considered all the above points. I tested the potential performance improvements from removing LINQ from critical loops, and the frequency of throwing Exceptions. I'm satisfied that the performance improvements aren't significant enough to warrant changes here.

With regard to the points about using an appropriate timer library, and making the interface implementations implicit, I agree and have adopted these changes. These can be found in this PR and will be available in v0.4.4.

Thanks for your time, I hope your dependency resolver is a great success.

[lifebeyondfife]

Thought about it for a while in the background, and can imagine use cases for small/simple constraint problems where solutions will be fast. Exceptions can be a bit of an anti-pattern in general when you consider some languages don't use them e.g. Golang. Decided to heed your advice @Jimmacle and refactor the search routine to manage flow control with boolean logic rather than throwing exceptions.