JuliaCollections / DataStructures.jl

Julia implementation of Data structures
https://juliacollections.github.io/DataStructures.jl/latest/
MIT License
679 stars 242 forks source link

fill!(::CircularBuffer) has a surprising definition #447

Open cstjean opened 5 years ago

cstjean commented 5 years ago

The definition of fill!(::CircularBuffer, val)

    fill!(cb, data)
Grows the buffer up-to capacity, and fills it entirely.
It doesn't overwrite existing elements.

is at odds with the Base definition

  fill!(A, x)

  Fill array A with the value x. If x is an object reference, all elements
  will refer to the same object. fill!(A, Foo()) will return A filled with the
  result of evaluating Foo() once.

When I think of the English word "fill", the CircularBuffer definition makes more sense, but clearly it's too late to change Base, and it seems bad to have the same function with two different meanings. I propose that, by analogy with isempty/empty!, we could rename the above fill! definition to full!, to match with isfull. @femtotrader

oxinabox commented 5 years ago

I see what you are saying. I initially wrote 90% of a reply disagreeing.

It does still do well to considers the difference between a CircularBuffer and a CircularDeque.

But I think you are right. Full is awkward though. We really want a verb. filltocapacity!? It is long but that isn't bad for a fairly rare op.

oxinabox commented 5 years ago

I'm still thinking about this. I went to use a CircularBuffer today and this was the behavour I expected.

I guess it depends on if you are thinking of your buffer as something that will mostly be empty, or something that will mostly be full.