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

feat(examples): Implement p/demo/dequeue #2170

Open
wants to merge 37 commits into
base: master
Choose a base branch
from

Conversation

linhpn99
Copy link
Contributor

@linhpn99 linhpn99 commented May 23, 2024

From issue : #2119

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@linhpn99 linhpn99 requested review from a team as code owners May 23, 2024 08:57
@linhpn99 linhpn99 requested review from ajnavarro and deelawn and removed request for a team May 23, 2024 08:57
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label May 23, 2024
@linhpn99
Copy link
Contributor Author

@moul i have just implemented queue structure, please take a look !

// PushFront adds an element to the front of the queue.
// Returns an error if the queue is full.
func (q *Queue) PushFront(value interface{}) error {
if q.begin-1 == q.end {
Copy link
Member

@moul moul May 26, 2024

Choose a reason for hiding this comment

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

With q.begin (uint) set to 0 by default, you may encounter an issue here.

Copy link
Contributor Author

@linhpn99 linhpn99 May 27, 2024

Choose a reason for hiding this comment

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

I have tested this case, q.begin will equal max.Uint64 and does not cause any other runtime errors

type Queue struct {
begin uint64 // Index of the front element
end uint64 // Index after the last element
data map[uint64]interface{} // Storage for queue elements
Copy link
Member

Choose a reason for hiding this comment

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

Using an AVL tree with SeqID appears preferable for optimizing storage efficiency at the moment.

@moul
Copy link
Member

moul commented May 26, 2024

You are adding general-purpose libraries, which is good. However, it's challenging to review without a real usage example. Could you create dapps using your new libraries or provide more actionable examples?

@linhpn99 linhpn99 requested a review from moul May 27, 2024 02:33
Copy link
Contributor

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

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

This data structure is not a Queue but a Deque (double-ended queue). Can you change the name to avoid confusion? Thanks.

@linhpn99 linhpn99 changed the title feat(examples): Implement p/demo/queue feat(examples): Implement p/demo/dequeue Jun 25, 2024
@linhpn99
Copy link
Contributor Author

This data structure is not a Queue but a Deque (double-ended queue). Can you change the name to avoid confusion? Thanks.

72cbc36
Thank for this suggestion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: In Progress
Status: In Review
Development

Successfully merging this pull request may close these issues.

None yet

3 participants