PermuteMultiplicationTable, refactor OnMultiplicationTable, add docs#1054
Conversation
|
Hopefully all addressed now @james-d-mitchell. I've also added some tests. |
james-d-mitchell
left a comment
james-d-mitchell
left a comment
There was a problem hiding this comment.
Looks generally great, a number of minor things to adjust, and then one major one: I'd suggest implementing PermuteMultiplicationTableNC in the C code which would not do any of the checks at the start of the current PermuteMultiplicationTable, just assume that they hold. Then modify PermuteMultiplicationTable to call that function after doing the checks. You could add assertions to PermuteMultiplicationTableNC to check that the requirements are met, when compiled without --enable-debug these checks will not be performed.
james-d-mitchell
added
the
waiting-for-creator-input
|
Not sure I've done exactly what you wanted with the assertions @james-d-mitchell but let me know |
Probably easiest for me to just do it, when you are done, please let me know and I'll add the assertions. |
Sounds good, all done now @james-d-mitchell |
|
Ah you did exactly what I was suggesting already, thanks! |
|
Just rebased so hopefully CI is all good now |
pramothragavan
force-pushed
the
OnMultiplicationTable
branch
from
af1bee6 to
54ec96f
Compare
…emigroups#1054) * PermuteMultiplicationTable, refactor OnMultiplicationTable, add docs * Addressed comments, added tests * clang format * clang-format * clang-format * address comments * Int_ObjInt -> INT_INTOBJ * more Int_ObjInt -> INT_INTOBJ * add asserts * fix ci, add more asserts * refactor * error messages
2 participants
This PR
PermuteMultiplicationTable(temp, table, p), implemented in C, which permutes a multiplication tabletableand stores the result by changingtempin-place.OnMultiplicationTableto callPermuteMultiplicationTable.I wasn't sure where to put the function, so I added a new file.