Conversation
|
Hey I really like the changes, and I'm glad someone is interested as I am in the project. One point - I've made simple UnitTest - just to make sure nothing is messed up. If you add some functionality, can you make simple unit test for it as well. If you change the naming, the usage in UnitTest should obviously also be changed. |
|
OK I've merged the branches. I did similar optimization in the Target search - so I left my code that I know works. The GetBoundary is not optimal, I'm now returning the code to use min-max optimization. The min and max are now made sure to be two cells from the last living cell (or at 0, N-1). So I'm not sure how performance sensitive is GetBoundary, but consider using min-max properties if you want to use it in search on each iteration. Everything else looks great - I also added small unit test for GetBoundary, and added prints. Also made sure the samples are working (I needed conio for getch, but getchar works as well). |
LifeAPI looks great. I hacked around with it a bit. What do you think of these patches?
Patch 1. IMO these headers do not really belong inside LifeAPI.h. Especially comio.h seems not available on Linux.
Patch 2. I hate dealing with strings in C/C++, IMO they should only be used for input/output.
Patch 3. gcc spews a load of warnings unless char * is declared as const
Patch 4. Direct implementations of Contains and ContainsInverse. I don't know if you will like them.
Patch 5. I think these renamings make it more obvious what the function does without needing to read the documenatation. Especially ContainsInverse I think is a tricky name that I would prefer to get rid of. This is probably the most subjective patch I am submitting.
Patch 6. Add function GetBoundary. Completely untested but could be useful to easily defined the "unwanted" set in a LifeTarget.
(Oh I notice you made some changes in the last couple of hours. Hopefully this still applies cleanly?! Maybe not)