(queue [...]) functions (#CLJ-1078)

161 views
Skip to first unread message

John Jacobsen

unread,
May 26, 2013, 9:51:26 AM5/26/13
to cloju...@googlegroups.com
The other day I was looking for a way to try out a PersistentQueue and found there wasn't any analogue of vec, list, etc. for queue.  One has to, e.g., 

    (into clojure.lang.PersistentQueue/EMPTY [1 2 3])

rather than

    (queue [1 2 3])

I found Timothy Baldridge's patch [1] which implements queue, queue* and queue?.  However, his queue works like (queue 1 2 3) and Rich has indicated on the ticket, "queue should take a collection rather than vec and sec".   This seemed straightforward to me, so I made some changes for review[2] in a fork on GitHub to adopt Timothy's changes to take a collection argument.

I can easily upload a patch to JIRA, but since this is my first patch, I thought I'd do a sanity check here to see if I'm barking up the right tree (this is my first contribution to the core language).

Thanks!
John Jacobsen

Mikera

unread,
May 27, 2013, 8:28:46 PM5/27/13
to cloju...@googlegroups.com
Looks like the right general approach to me.

Shouldn't "queue?" test against an interface rather than a concrete class, in case people decide to implement other types of queue in the future?

John Jacobsen

unread,
May 27, 2013, 10:52:17 PM5/27/13
to cloju...@googlegroups.com
On Monday, May 27, 2013 7:28:46 PM UTC-5, Mikera wrote:
Looks like the right general approach to me.

Shouldn't "queue?" test against an interface rather than a concrete class, in case people decide to implement other types of queue in the future?

I'd be OK with that... but there is no such interface yet and I suppose we'd want a few more +1 votes before going down that road.  In the mean time one possibility would be to just proceed with this modest change and, if someone else got around to implementing another kind of queue, generalize "queue?" to the more abstract interface at that time.  Any thoughts on that?

I'll wait another day or three, and, if there are no other comments, upload the patch to JIRA for the screeners to follow up on.

John

Michał Marczyk

unread,
Jul 11, 2013, 12:04:16 AM7/11/13
to cloju...@googlegroups.com
On 28 May 2013 04:52, John Jacobsen <eigen...@gmail.com> wrote:
> On Monday, May 27, 2013 7:28:46 PM UTC-5, Mikera wrote:
>>
>> Looks like the right general approach to me.
>>
>> Shouldn't "queue?" test against an interface rather than a concrete class,
>> in case people decide to implement other types of queue in the future?
>
>
> I'd be OK with that... but there is no such interface yet and I suppose we'd
> want a few more +1 votes before going down that road.

Here's one. For example, a persistent deque implementation would
probably be interested in supporting the regular queue API (though see
below for a problem with this).

There's at least one WIP deque impl already:

https://github.com/pjstadig/deque-clojure

One thing which comes to my mind is that while, say, RRB vectors could
act as persistent queues (and deques), they probably won't be able to
share API with c.l.PersistentQueue, as it is actually shared between
the persistent queue and the persistent stacks (vectors and lists).
It's not just about the Java-level API either, it's about peek and
pop. Actually this also applies to any deque impl. Hm.

> In the mean time one
> possibility would be to just proceed with this modest change and, if someone
> else got around to implementing another kind of queue, generalize "queue?"
> to the more abstract interface at that time. Any thoughts on that?

Disregarding the previous paragraph for a moment, it seems to me that
(1) checking for IPersistentQueue (IQueue in ClojureScript style?) is
the correct thing to do, (2) introducing the interface alongside
queue? makes the patch more valuable (because it would create a
well-defined interface for alternative impls to participate in).

Cheers,
Michał


>
> I'll wait another day or three, and, if there are no other comments, upload
> the patch to JIRA for the screeners to follow up on.
>
> John
>
>>
>>
>> On Sunday, 26 May 2013 21:51:26 UTC+8, John Jacobsen wrote:
>>>
>>> The other day I was looking for a way to try out a PersistentQueue and
>>> found there wasn't any analogue of vec, list, etc. for queue. One has to,
>>> e.g.,
>>>
>>> (into clojure.lang.PersistentQueue/EMPTY [1 2 3])
>>>
>>> rather than
>>>
>>> (queue [1 2 3])
>>>
>>> I found Timothy Baldridge's patch [1] which implements queue, queue* and
>>> queue?. However, his queue works like (queue 1 2 3) and Rich has indicated
>>> on the ticket, "queue should take a collection rather than vec and sec".
>>> This seemed straightforward to me, so I made some changes for review[2] in a
>>> fork on GitHub to adopt Timothy's changes to take a collection argument.
>>>
>>> I can easily upload a patch to JIRA, but since this is my first patch, I
>>> thought I'd do a sanity check here to see if I'm barking up the right tree
>>> (this is my first contribution to the core language).
>>>
>>> Thanks!
>>> John Jacobsen
>>>
>>> [1] http://dev.clojure.org/jira/browse/CLJ-1078
>>> [2]
>>> https://github.com/eigenhombre/clojure/compare/master...tweak-queue-impl
>
> --
> You received this message because you are subscribed to the Google Groups
> "Clojure Dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to clojure-dev...@googlegroups.com.
> To post to this group, send email to cloju...@googlegroups.com.
> Visit this group at http://groups.google.com/group/clojure-dev?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
Reply all
Reply to author
Forward
0 new messages