Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External stats change #26

Closed
wants to merge 3 commits into from
Closed

External stats change #26

wants to merge 3 commits into from

Conversation

MaxOstrowski
Copy link
Member

Merging UserStatistics and ClaspStatistics for uniform C interface preparation.
In this way the clingo statistics interface can be reused and only one interface has to exist.

@BenKaufmann
Copy link
Contributor

Hi Max,

I really like the idea of combining the interfaces. However, one golden rule of interface design is that interfaces should be easy to use correctly and hard to use incorrectly. Imo, your solution is too easy to use incorrectly. Most clasp statistics for example are read-only StatisticObjects and calling add on a key belonging to such a statistic will trigger undefined behaviour - in the best case you'll get an immediate seg fault but worse consequences are easily possible. To make the interface at least safely usable the following is needed:

  • a function extensible(key) indicating whether or not key can be extended
  • a (checked) precondition for add(key, ...) enforcing that extensible(key) == true
  • a root-key for which extensible(key) == true

The implementation of the combined interface in clasp should probably also avoid storing (user) statistics twice (once in the map and once in the objects vector).

I'll try to implement something like that over the weekend...

@BenKaufmann
Copy link
Contributor

Hi Max,
I updated #23 accordingly. Please have a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants