Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Peek API to view data of buffer without popping #6309

Merged
merged 2 commits into from Mar 22, 2018

Conversation

deepikabhavnani
Copy link

@deepikabhavnani deepikabhavnani commented Mar 8, 2018

Description

Added peek API to view data of CircularBuffer without popping.

Pull request type

  • Fix
  • Refactor
  • New target
  • Feature
  • Breaking change

@geky
Copy link
Contributor

geky commented Mar 8, 2018

I believe this falls under "feature"

Does the Circular Buffer have tests? Could we add a test covering this function?

* @param data Data to be peeked from the buffer
* @return True if the buffer is not empty and data contains a transaction, false otherwise
*/
bool peek(T& data) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, does data need to be const if peek is a const member function?

Copy link
Author

Choose a reason for hiding this comment

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

const member function is not allowed to modify the object elements, which is not done in case of this API. But we are passing data as return value hence it cannot be const.

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be an issue if it was a const reference (T& const data). But since it's a reference-to-const (const T& data), it should be fine.

Does this compile as is with a const CircularBuffer?

Copy link
Author

Choose a reason for hiding this comment

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

Not all members of CircularBuffer are const, without this API also it won't compile as const CircularBuffer.

(const T& data) in case of template class says value of data variable cannot be modified in the function. We are passing the data value, hence it cannot be const. I tried compilation with const and it failed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, that's annoying if there's a similar issue in other functions.

const T& data can be assigned with issue.

Copy link
Contributor

@cmonr cmonr left a comment

Choose a reason for hiding this comment

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

LGTM

@cmonr
Copy link
Contributor

cmonr commented Mar 21, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Mar 21, 2018

Build : SUCCESS

Build number : 1503
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6309/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Mar 21, 2018

@mbed-ci
Copy link

mbed-ci commented Mar 21, 2018

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 21, 2018

/morph export-build

@mbed-ci
Copy link

mbed-ci commented Mar 21, 2018

@cmonr cmonr merged commit 9932b4c into ARMmbed:master Mar 22, 2018
@deepikabhavnani deepikabhavnani deleted the peek_api branch March 22, 2018 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants