3
\$\begingroup\$

Does the implementation look correct? Did I miss something major/minor?

class CircularBuffer

  class BufferEmptyException < StandardError;  end;
  class BufferFullException < StandardError;   end;

  def initialize(size)
    @size = size
    @buffer = []
  end

  def write(data)
    raise CircularBuffer::BufferFullException if @buffer.size >= @size    
    @buffer << data if data
  end

  def write!(data)
    if data
      @buffer << data
      @buffer.shift
    end  
  end

  def clear
    @buffer = []
  end  

  def read
    data = @buffer.shift
    raise CircularBuffer::BufferEmptyException unless data
    data
  end

end
\$\endgroup\$
4
\$\begingroup\$
  • I can pass the initializer a negative size. Or a non-numeric size. I won't get any complaints until I try to use the buffer.

  • If I use write! exclusively, the buffer will always be empty. You're always shifting an element off of the array - even if it's the one you just added.

  • It's impossible to write nil or false to the buffer, though either of those are values someone might legitimately want to write.

  • But even if you manage to write a false'y value, you can't get it back; you'll just get a BufferEmptyException even if the buffer's not empty. Raise if the array's truly empty, and do so before you shift; not if the value you already shifted off is false'y. And, worse yet, the value is just gone forever. You shifted it off the array, so it's just disappeared. Since you can just use the array's length to tell you the state of the buffer, it's no problem to store false'y values.

  • It's generally good practice to define a non-bang method in terms of its bang counterpart. Right now you have 2 separate places where things are added to the buffer, instead of having one method call the other, just with an extra check.

  • Speaking of, it'd be nice to have a bang version of read that doesn't raise exceptions.

  • It'd be nice to have an attr_reader for the size, and a to_a for the items.

  • It'd also be nice to have a full? method. Otherwise you'll always have to either rescue an exception from write, or exclusively use write!. Besides, you could use the same full? method within the class too.

  • Likewise, an empty? method would be nice to complement the full? method. While you're at it, you might even add a method that returns the number of slots left in the buffer, and/or one that that returns the number of slots occupied.

Of course, it's not quite a circular buffer since, for one, it's not a perfectly fixed size. If write! worked as intended, and only shifted off items when necessary, it'd still (in it's current form) shift them off only after the buffer's "overflowed".

For another, it's not actually circular. You don't end up overwriting previously set slots. Instead you push and shift and array that expands up to a certain size. From the outside, it might well look like a circular buffer, but none of its internals match that description.

What you have is a general (almost) fixed-length (or rather "limited-length") FIFO buffer, but I wouldn't call it circular. But then again, implementing a strictly circular buffer in Ruby isn't really that useful. A FIFO buffer has its uses, sure, but a strictly circular one seems like trying to do low-level programming in a very high-level language. It's a technique that's useful when you have to allocate memory, and it's cool when you can do pointer arithmetic. But you don't allocate memory in Ruby, and you can't do pointer manipulation, so the arguments and tools for creating efficient buffers aren't really there.

It's like camping in you own back yard. It's fun to try, but it's not survivalism (and it'd probably be kinda dangerous to make a campfire).

But hey, it is fun

class FixedLengthFIFOBuffer
  class BufferEmptyException < StandardError;  end;
  class BufferFullException < StandardError;   end;

  attr_reader :size

  def initialize(size)
    @size = size.to_i # still lacking a check for values below 1
    @buffer = []
  end

  def full?
    @buffer.size >= size
  end

  def empty?
    @buffer.empty?
  end

  def write(data)
    raise CircularBuffer::BufferFullException if full?
    write! data
  end

  def write!(data)
    @buffer.shift if full? # maybe use read! instead and return the value?
    @buffer << data
  end

  def clear
    @buffer.clear
  end

  def read
    raise CircularBuffer::BufferEmptyException if empty?
    read!
  end

  def read!
    @buffer.shift
  end

  def to_a
    @buffer.dup
  end
end
\$\endgroup\$
  • \$\begingroup\$ I understand regarding high/low languages, but to learn concepts thoroughly, you must implement them and even rubyists need to learn DS. \$\endgroup\$ – archie Oct 21 '14 at 22:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.