Closed NdubuisiJr closed 4 years ago
changed the '{' to match the repo's convention. Implemented our suggestions on the review comments.
Fixed failing unit test
Thanks, looks good, great job!
Please refer to this Wikipedia article, specially the reference to Read and Write pointers: Circular Buffer.
Concerning the Read and Write pointers. The additional mechanism used for checking the positions of the start and end indexes when using two pointers talked about in this article Circular_buffer. The article suggested that the buffer should hold (size - 1) items. That is what I actually did In the constructor when instantiating the array _circularBuffer = new T[length + 1];
. But Instead of decreasing the overall size I just increased it by 1. The logic works the same way.
Implemented modification
Concerning the Read and Write pointers. The additional mechanism used for checking the positions of the start and end indexes when using two pointers talked about in this article Circular_buffer. The article suggested that the buffer should hold (size - 1) items. That is what I actually did In the constructor when instantiating the array
_circularBuffer = new T[length + 1];
. But Instead of decreasing the overall size I just increased it by 1. The logic works the same way.
Thanks for implementing the changes, the code looks much better now 👍
The reason I referenced the article, which maybe I should have made more explicit, is the fact that a Circular Buffer would allow you to manipulate the write and read indexes separately which will make it a building block for other circular data structures, i.e.: Circular Queues and Circular Stacks, but since you don't provide that in your implementation we will have to make it a higher level abstraction, i.e.: Circular Stack, or we agree that you need to implement a lower level Circular Buffer which would then be used as a building block for implementing other Circular Linear Collections (e.g.: CQueues and CStacks).
Btw, take a look at PR #80 which supposedly implements a Circular Queue, it won't be possible to have two implementations of the same exact thing.
Done. Also, added a test method to very the methods in ICollection
interface
Please Review the current state
@NdubuisiJr - the data structure looks much better and more polished now. I have one remaining question though. The Remove
method allows the user to remove a single element from the buffer, which would leave the buffer with a non-existent extra item assigned to the same slot in the array (default(T)
), I think this might be a problem, here is why:
Let's say the buffer contains 5 elements
| 1 | 2 | 3 | 4 | 5 |
And then, I choose to delete 4
, the buffer will assign the default value of Integers, which is 0
, in that slot:
| 1 | 2 | 3 | *0* | 5 |
This is a problem because the buffer now has one spot free but new additions will over-write the first element instead of the free one.
I think a better implementation of Remove
would actually shift all elements after the one you just removed one slot to the back, and then update the _end
pointer to reference the new slot where the end element is, leaving buffer with an open slot at the end:
| 1 | 2 | 3 | 5 | *0* |
This is better because new additions can start writing at the end of the buffer, appropriately before moving all the way to the front and start overwriting the older elements.
I think this will leave us with a better implementation but will make things hairier, i.e.:
Array.copyTo
API to copy things over and then discard the existing one?What do you think?
@aalhour, I really want to appreciate you and @PatrykOlejniczak for the reviews so far. I've really learned much from you guys.
I think we should go for the first option you gave because it reduces the number of loops and if
checks.
Used a while loop to move each element after the deleted one.
Please review the current state. Thanks.
Checkout the circular Buffer Data structure. Added unit tests too.