Proposal for SiteSet simplification. #404
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
-The design need the behaviour of an array of polymorphic objects. If you try to do this using any pointers/new/delete then you fighting the language. I have also tried to do this in the past. -In ordfer to work with the language we must use new when creating sites, but those pointers cab be immediately deposited in a std::shared_pointer<SiteBase>. This is a reference counted pointer that does all the memory management for us. AND we can copy them (or the whole array) at will. -Tested for memory leaks with valgrind .. no leaks. -Users adding new site types will have to virtually inheret from SiteBase. But I don't think they will need to directly deal with pointers. You can see and example of this in the sample/electronk.h -I think we can set things up so that users can build arbitrary mixed site sets. again without using pointers directly. -In the unit tests I added some examples of what can happen when writeing to file and reading back. Right now you must know what SiteSet type you are reading back other wise chaos will ensue! Again we can fix this using a factory pattern behind the scenes. SiteSet refactoring proposal Fix build in sample folder
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
-The design need the behaviour of an array of polymorphic objects. If you try to do this using any pointers/new/delete then you fighting the language. I have also tried to do this in the past.
-In ordfer to work with the language we must use new when creating sites, but those pointers cab be immediately deposited in a std::shared_pointer. This is a reference counted pointer that does all the memory management for us. AND we can copy them (or the whole array) at will.
-Tested for memory leaks with valgrind .. no leaks.
-Users adding new site types will have to virtually inheret from SiteBase. But I don't think they will need to directly deal with pointers. You can see and example of this in the sample/electronk.h
-I think we can set things up so that users can build arbitrary mixed site sets. again without using pointers directly.
-In the unit tests I added some examples of what can happen when writeing to file and reading back. Right now you must know what SiteSet type you are reading back other wise chaos will ensue! Again we can fix this using a factory pattern behind the scenes.
SiteSet refactoring proposal
Fix build in sample folder