NULLx76 / ringbuffer

A fixed-size circular buffer written in Rust.
https://crates.io/crates/ringbuffer
MIT License
95 stars 20 forks source link

default initialization #50

Closed jdonszelmann closed 3 years ago

jdonszelmann commented 3 years ago

adds an init_default method which initializes the full ringbuffer with default elements. It's a method so you can choose what kind of backing buffer there will be beforehand (avoiding the need for a number of methods like init_default_with_capacity and init_default_new etc).

Example usage:

let mut rb = AllocRingBuffer::with_capacity(32);
rb.fill_default();

Note: you can't do:

let mut rb = AllocRingBuffer::with_capacity(32).fill_default();

because that'd mean we'd need to put a Self:Sized bound on init_default(). That's fine in this case, but without the bound you can also later reuse init_default() to clear the ringbuffer and re-initialize it.

codecov[bot] commented 3 years ago

Codecov Report

Merging #50 (4f71f9f) into master (d43858d) will increase coverage by 0.08%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
+ Coverage   98.35%   98.44%   +0.08%     
==========================================
  Files           4        4              
  Lines         731      773      +42     
==========================================
+ Hits          719      761      +42     
  Misses         12       12              
Impacted Files Coverage Δ
src/lib.rs 100.00% <100.00%> (ø)
src/ringbuffer_trait.rs 98.82% <100.00%> (+0.05%) :arrow_up:
src/with_alloc.rs 94.73% <100.00%> (+0.29%) :arrow_up:
src/with_const_generics.rs 93.97% <100.00%> (+0.30%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d43858d...4f71f9f. Read the comment docs.

jdonszelmann commented 3 years ago

@phip1611 I can't mark you for review, but do you still want to take a look if it covers your usecases?

NULLx76 commented 3 years ago

It might be nice to make it init_with(T)instead though not sure if this makes it too configurable and if its desirable to change the bound of T from Default to Clone

NULLx76 commented 3 years ago

It might also be nice to add an example usage to the docs of the function.

(Github mobile doesn't allow me to comment on the code itself)

jdonszelmann commented 3 years ago

we can't call it init_with as with generally means it takes a closure (ie init_with(|| Default::default()))

phip1611 commented 3 years ago

I can't mark you for review, but do you still want to take a look if it covers your usecases?

Yep, looks like what I need.

NULLx76 commented 3 years ago

we can't call it init_with as with generally means it takes a closure (ie init_with(|| Default::default()))

Then just init(with: T) perhaps? But whatever it's called doesn't ultimately matter, just if we prefer it over filling with default

jdonszelmann commented 3 years ago

@phip1611 use version 0.8.1 (just published)