Split storm-dft.h into analysis, io and transformation - #1034
Conversation
| @@ -1,26 +1,46 @@ | |||
| #include "storm-dft/api/storm-dft.h" | |||
| #include "analysis.h" | |||
There was a problem hiding this comment.
wrong naming scheme? Or is this the new naming scheme?
There was a problem hiding this comment.
To be honest, I thought this was the new naming scheme. But I am not sure whether we agreed on it.
| * @param dft DFT. | ||
| * @return Pair of GSPN and id of failed place corresponding to the top level element. | ||
| */ | ||
| template<typename ValueType> |
There was a problem hiding this comment.
The transform to GSPN/JANI feel a bit like the odd-ones-out. They are more like conversion compare do the other questions?
There was a problem hiding this comment.
(and they drag in a lot of headers)
There was a problem hiding this comment.
I could split it into translation and transformation. Maybe call it gspn_translation to avoid confusion.
| #include "storm-dft/utility/RelevantEvents.h" | ||
| #include "storm-parsers/api/properties.h" | ||
| #include "storm/api/properties.h" | ||
| #include "storm/storage/jani/Property.h" |
There was a problem hiding this comment.
It is a bit ugly that you need to invoke three proper-related headers :-)
There was a problem hiding this comment.
Yes, but this is a separate issue in my view. In particular the inclusion of jani/Property is very ugly because it is only forward declared otherwise.
|
LGTM! Should also help compile times?! |
|
I briefly looked into the storm-dft/storage/Dft.h. There were some headers like layoutinfo that may be removed via forward refs and there are a few implementations in the header that may be able to go to the cpp (and then there may be more headers to move out of that?) I looked at it as it is basically the main header that the dft-tests import via io. |
|
I did not measure the compile times so far but it should be better encapsulated now. |
2 participants
storm-dft.hintoanalysis.h,io.handtransformation.h