Skip to content

Conversation

@iseletsk
Copy link

@iseletsk iseletsk commented Nov 2, 2020

I am not sure you would want to merge this in - this adds a prepend functionality that I need for another project. I am sure it is not the best way to do prepend - but I expect the operation to be really infrequent.
I totally understand if you don't want it as part of your dque package as it breaks size functionality, and some of the "expectations"

  1. All objects will be added into a new single segment, and that segment will be the first segment (so, don't prepend too much in one goo)
  2. Size will provide incorrect output

…ront of the queue.

When using prepend functionality, expect that:
1. All objects will be added into a new single segment, and that segment will be the first segment (so, don't prepend too much in one goo)
2. Size will provide incorrect output
@joncrlsn
Copy link
Owner

joncrlsn commented Nov 6, 2020

Igor, Thank you for your PR. What is your use case? Is it so you can take something off the queue, then add it back near the front of the queue if you are not able to successfully handle it?

@iseletsk
Copy link
Author

iseletsk commented Nov 6, 2020 via email

@joncrlsn
Copy link
Owner

joncrlsn commented Nov 6, 2020

I like the idea, and I think it is needed. It is a better idea than locking the queue while you process the item.

However, for the implementation I'd rather see is a qSegment (with number zero) that contains all the prepended items that the Dequeue method would check first. Then the Size() method could be made to work right. That would take some more coding to clean it out occasionally, which I could help with.

@iseletsk
Copy link
Author

iseletsk commented Nov 6, 2020 via email

@joncrlsn
Copy link
Owner

joncrlsn commented Nov 9, 2020

You could require people to enable the prepend functionality. That would eliminate the two reads for the people who don't use prepend.

Good thought. I'm wondering how important it is to put 1, 2, and 3 back in the right order. If you weren't able to handle it the first time (i.e. database unavailable), it's unlikely that an immediate second try would work either. In that case, putting it back to the end of the zero queue might be a good thing.

But I'm thinking it is going to be necessary to prepend with a wait period so you don't end up in an infinite loop handling things that not are working, while never getting to the new work in queues 1+.

@iseletsk
Copy link
Author

iseletsk commented Nov 9, 2020 via email

@joncrlsn
Copy link
Owner

joncrlsn commented Nov 11, 2020

Hi Igor, I've started thinking that new functionality like this should be in a wrapper/decorator object that enhances the current queue object without changing it. That would allow different features based on which wrapper/decorator you choose.

I would add links to the README documentation for projects that are wrapper/decorator objects.

https://en.wikipedia.org/wiki/Decorator_pattern

Thoughts?

@iseletsk
Copy link
Author

iseletsk commented Nov 11, 2020 via email

@joncrlsn
Copy link
Owner

My experience with OO is from Smalltalk and Java, so hopefully my terms will match yours enough.

You are right that it would not be a true decorator because it will actually extend the API with a new Prepend method that is in your wrapper class, and pass the other methods through to the Dque instance.

We'd probably need to make the qSegment class/struct public/external (first letter uppercase) so you can use that.

You might also need to add some methods to the wrapped DQue class/instance/struct that would allow you to integrate with the internals better. But you should try to just use what you have first, even if that means using your own mutexes in the wrapper class (might work depending on your implementation). Any new methods on Dque would need to be public if your wrapper is in it's own package. But could be private (starts with lowercase) if we include the decorator in the dque package.

Let me know if I didn't explain everything as well as I could.

I hope that's not too hard, but if it is, and you need the functionality as soon as possible, you could fork the project (as you've done) and make the changes in your own project, and use that.

@joncrlsn
Copy link
Owner

joncrlsn commented Nov 11, 2020

Using composition to add features usually adds some effort, but it pays off in being easier to test and allowing multiple implementations you can choose from (depending on your use-case).

@iseletsk
Copy link
Author

iseletsk commented Nov 11, 2020 via email

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.

3 participants