Skip to content

added boolean helpers to check buffer state#5

Open
Sreshty-Beshty wants to merge 1 commit intoLearnPRG-py:mainfrom
Sreshty-Beshty:main
Open

added boolean helpers to check buffer state#5
Sreshty-Beshty wants to merge 1 commit intoLearnPRG-py:mainfrom
Sreshty-Beshty:main

Conversation

@Sreshty-Beshty
Copy link
Copy Markdown

solves Issue: #3

Comment thread src/Vectorial.h
}
}

// checks if buffer is empty
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nit: remove comments

Comment thread src/Vectorial.h
return entriesAdded == 0;
}

// checks if buffer is full
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ditto

Comment thread src/Vectorial.h
// checks if buffer is full

bool is_full() {
return entriesAdded == N;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This would cause an overflow and cause this to be inaccurate

in push_back(), entries_added++ happens, hence it is possible in cycle mode that entriesAdded is actually > N. Maybe address this with >=

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

also size() exists in the same struct you may want to consider that as a safer approach that doesnt use >=

Comment thread src/Vectorial.h
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Add readme coverage maybe?

Comment thread src/Vectorial.h
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We'd also likely have to update examples perhaps. You can have comments there about calling the functions and their purpose.

@LearnPRG-py
Copy link
Copy Markdown
Owner

solves Issue: #3

  1. Make this fixes: [CORE] Standard isFull() and isEmpty() #3 so it auto-resolves on close
  2. Add more description on the change itself ("This PR adds 2 bool functions is_empty() and is_full()...")

@LearnPRG-py
Copy link
Copy Markdown
Owner

Also there's conflicts now: have fun with that :)

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