{"action":"created","issue":{"url":"https://api.github.com/repos/rlogiacco/CircularBuffer/issues/16","repository_url":"https://api.github.com/repos/rlogiacco/CircularBuffer","labels_url":"https://api.github.com/repos/rlogiacco/CircularBuffer/issues/16/labels{/name}","comments_url":"https://api.github.com/repos/rlogiacco/CircularBuffer/issues/16/comments","events_url":"https://api.github.com/repos/rlogiacco/CircularBuffer/issues/16/events","html_url":"https://github.com/rlogiacco/CircularBuffer/pull/16","id":389055061,"node_id":"MDExOlB1bGxSZXF1ZXN0MjM3MTUxMjM3","number":16,"title":"Calling element constructors/destructors","user":{"login":"Erlkoenig90","id":7581656,"node_id":"MDQ6VXNlcjc1ODE2NTY=","avatar_url":"https://avatars3.githubusercontent.com/u/7581656?v=4","gravatar_id":"","url":"https://api.github.com/users/Erlkoenig90","html_url":"https://github.com/Erlkoenig90","followers_url":"https://api.github.com/users/Erlkoenig90/followers","following_url":"https://api.github.com/users/Erlkoenig90/following{/other_user}","gists_url":"https://api.github.com/users/Erlkoenig90/gists{/gist_id}","starred_url":"https://api.github.com/users/Erlkoenig90/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Erlkoenig90/subscriptions","organizations_url":"https://api.github.com/users/Erlkoenig90/orgs","repos_url":"https://api.github.com/users/Erlkoenig90/repos","events_url":"https://api.github.com/users/Erlkoenig90/events{/privacy}","received_events_url":"https://api.github.com/users/Erlkoenig90/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":13,"created_at":"2018-12-09T19:27:05Z","updated_at":"2018-12-12T07:57:39Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/rlogiacco/CircularBuffer/pulls/16","html_url":"https://github.com/rlogiacco/CircularBuffer/pull/16","diff_url":"https://github.com/rlogiacco/CircularBuffer/pull/16.diff","patch_url":"https://github.com/rlogiacco/CircularBuffer/pull/16.patch"},"body":"This PR lets the buffer automatically call constructors/destructors of elements properly, fixing #12 (if I understood it correctly). This allows users to place class objects in the buffer which do something in these functions, e.g. allocate memory in the c'tor and free it in the d'tor; with this PR, memory is freed correctly when the element is removed.\r\nThis is achieved by placing the elements inside a `union`, which does not call constructors/destructors automatically. There is however an AVR-specific problem: For calling the constructor explicitly, the placement-new syntax has to be used. For this to work, an overload for `operator new` is required, which is provided by the `<new>` header. Since AVR-GCC does not ship with standard c++ headers, we have to provide our own overload. It is just one line, but that might collide with other libraries which do the same. Therefore, I added a comment to have the user comment out the line in case of a problem. This is only done for AVR, as the ARM-GCC does provide `<new>`.\r\nThe parameter type for `unshift` and `push` is now an \"[universal reference](https://isocpp.org/blog/2012/11/universal-references-in-c11-scott-meyers)\" to allow passing arbitrary objects to the element constructor.\r\nThe additions should not incur any overhead for types with trivial c'tors/d'tors such as integers. The code compiles, but I did not test it on actual hardware."},"comment":{"url":"https://api.github.com/repos/rlogiacco/CircularBuffer/issues/comments/446495617","html_url":"https://github.com/rlogiacco/CircularBuffer/pull/16#issuecomment-446495617","issue_url":"https://api.github.com/repos/rlogiacco/CircularBuffer/issues/16","id":446495617,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NjQ5NTYxNw==","user":{"login":"Erlkoenig90","id":7581656,"node_id":"MDQ6VXNlcjc1ODE2NTY=","avatar_url":"https://avatars3.githubusercontent.com/u/7581656?v=4","gravatar_id":"","url":"https://api.github.com/users/Erlkoenig90","html_url":"https://github.com/Erlkoenig90","followers_url":"https://api.github.com/users/Erlkoenig90/followers","following_url":"https://api.github.com/users/Erlkoenig90/following{/other_user}","gists_url":"https://api.github.com/users/Erlkoenig90/gists{/gist_id}","starred_url":"https://api.github.com/users/Erlkoenig90/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Erlkoenig90/subscriptions","organizations_url":"https://api.github.com/users/Erlkoenig90/orgs","repos_url":"https://api.github.com/users/Erlkoenig90/repos","events_url":"https://api.github.com/users/Erlkoenig90/events{/privacy}","received_events_url":"https://api.github.com/users/Erlkoenig90/received_events","type":"User","site_admin":false},"created_at":"2018-12-12T07:57:39Z","updated_at":"2018-12-12T07:57:39Z","author_association":"CONTRIBUTOR","body":"> I haven't had the time to test the changes, but I've been studying them and I'm having a hard time to understand what's going on... I feel like I'm back at school disappointed\r\n\r\nWell, that's the cool thing about C++, you can do so many crazy things that few people know 🤪\r\n\r\n>  apologize in advance for the time it will take to merge it, but I need to feel comfortable with the approach otherwise I'll not be able to maintain the library: hope you understand.\r\n\r\nHehe that's alright, there's no hurry. It's good for finding bugs. If you want to learn more about C++, there's a [list of good books](https://stackoverflow.com/a/388282).\r\n\r\n> The following code outputs plenty of compiler errors which I'm not even able to understand... cry\r\n\r\nOh no, I should not have removed the `union`'s constructors and destructors... I added them back. With this code, it should now work:\r\n\r\n```\r\n#include <CircularBuffer.h>\r\n#include \"Record.h\"\r\n\r\n#define SAMPLE_PIN A0\r\nvoid setup() {\r\n  pinMode(SAMPLE_PIN, INPUT);\r\n}\r\nCircularBuffer<Record, 100, byte> buffer;\r\n\r\nvoid loop() {\r\n  buffer.push(Record{analogRead(SAMPLE_PIN), millis()});\r\n  delay(50);\r\n  if (buffer.isFull()) {\r\n    while (!buffer.isEmpty()) {\r\n      buffer.pop();\r\n    }\r\n  }\r\n}\r\n```\r\n\r\nYou don't need `new`, as the objects are stored right inside the `CircularBuffer::buffer` array, and not on heap. The `Record{analogRead(SAMPLE_PIN), millis()}` creates a new temporary object (on stack) and passes it to `push`, which then creates a new object inside the `buffer` array (using placement-new syntax) and passes the temporary object to its copy-constructor, which was automatically defined by the compiler. This whole procedure is optimized by the compiler to not actually store a  temporary object on stack but put the integers in the buffer directly. It works very similar to storing simple integers inside the buffer - if you type `buffer.push(42)`, a temporary integer with 42 is allocated, passed to `push`, which then copies it into the `CircularBuffer::buffer` variable.\r\n\r\n500 `Record` objects don't fit in the RAM, because that requires (500 x (sizeof(unsigned int)+sizeof(unsigned long))) = 3000 bytes."}}
{"actor":{"display_login":"Erlkoenig90"}}