Skip to content

Comments

Update ring buffer exercise#159

Open
axel-van-abkoude wants to merge 8 commits intotrifectatechfoundation:mainfrom
axel-van-abkoude:update-ring-buffer
Open

Update ring buffer exercise#159
axel-van-abkoude wants to merge 8 commits intotrifectatechfoundation:mainfrom
axel-van-abkoude:update-ring-buffer

Conversation

@axel-van-abkoude
Copy link
Contributor

Additions include:

  • Including technically correct //! doc comments instead of ///
  • Removing the magic number 16 to a variable called RING_SIZE
  • Added function templates for has_room and peek()
  • Including some test cases to pass when completing the entire exercise

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this I wonder why we do not return a Result here? Was that not introduced yet in the material?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its introduced in 2/3/2-error-handling which is before this file. Although the function does not really return a value.

We could make it a Result<Void,Error>?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice, yes in Rust that would be written as Result<(), ()> or more generic Result<(), u8>, returning back the value that was passed in. This is less of a problem for a u8 since it is copied, but important for moved types like String.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented it as Result<(),&static' str> as it seemed nicer with a description of the error

Copy link
Collaborator

@tdittr tdittr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now :) just run cargo fmt once to straighten out the formatting.

// 5) EXTRA EXERCISES:
// - implement the method "has_room" so that "queue.has_room()" is true if and only if writing to the queue will succeed
// - implement the method "peek" so that "queue.peek()" returns the same thing as "queue.read()", but leaves the element in the queue
// - eliminate edge cases of the data structure (HINT: what is the behaviour of (n % 0))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice wording!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants