circular-buffer: Added template to the stub file#608
circular-buffer: Added template to the stub file#608petertseng merged 2 commits intoexercism:masterfrom
Conversation
| FullBuffer, | ||
| } | ||
|
|
||
| impl<T: Debug> CircularBuffer<T> { |
There was a problem hiding this comment.
same comment as with sublist - I know that this Debug constraint is added purely so that we can reference variables in unimplemented message. We made a different choice in c5f7a43 and I'll consider whether it's better to use underscore prefix here. Input appreciated.
There was a problem hiding this comment.
I think that we should not bound T: Debug here, and instead use an underscore variable. Perhaps it's worth identifying which of the exercises first uses an underscore variable in the track and adding a comment explaining why that is used, instead of a Debug trait bound.
| ); | ||
| } | ||
|
|
||
| pub fn write(&mut self, element: T) -> Result<(), Error> { |
There was a problem hiding this comment.
I think Result<(), Error> makes sense and is the accepted way, as https://doc.rust-lang.org/std/io/trait.Write.html#method.write_all does this as well. While it would be possible to return the index of the inserted value, I think the API presented in this exercise is not conducive to a client of this code being able to do anything with that information, so I wouldn't add it.
There was a problem hiding this comment.
Agree: Result<(), Error> is fine and idiomatic.
3 participants
Contributes to the #551
It is unclear what
writemethod should return on success, so the return type isResult<(), Error>.Perhaps
()could be replaced withusizeas to indicate the index of the successful write?