14:23:17moldybitsi had forgotten about equalp because i hadn't run into this yet
14:23:59pjbindeed. and the donwside of equalp is that: (equalp #(-3 1 "Hello") (vector -3 1 "HELLO")) #| --> t |#
14:25:03pjbSo you may have to write your own (defun vec-equal (a b) (and (= (length a) (length b)) (every (function equal) a b))) (vec-equal #(-3 1 "Hello") (vector -3 1 "HELLO")) #| --> nil |# (vec-equal #(-3 1 "Hello") (vector -3 1 "Hello")) #| --> t |#
18:49:08phoeI'd like one more review on https://github.com/sharplispers/split-sequence/pull/13 - I am in particular unsure about the two things sionescu has mentioned that I have not yet resolved.
18:49:26phoe1) The original code did not adhere to 80 chars per line - should I reformat it that way?
18:50:10phoe2) Should I drop the LIST branch of the typecase? My code does not traverse the whole list to determine its length, as per pjb's suggestion.
20:06:57phoeI think it's an iteration above what was before, but you make some good points
20:07:08phoeI just don't think my algorithmic skills are good enough to implement your final point
20:09:36sjlWriting code that works, doesn't cons excessively, and is backwards compatible with the existing behaviour is not as easy as it seems when you first see it.
20:10:20phoesjl: you made me remember the test suite of SPLIT-SEQUENCE
20:10:36phoebackwards compatibility with what the original code did caused my code to become the ugly monstrosity that it was
20:11:08phoeand, honestly, I invite anyone who's up for it to write better code that still conforms to the split-sequence test suite - it's a challenge
20:11:52fe[nl]ixat least we have an extensive test suite :)
20:12:07sjle.g. you have to stop *exactly* where the original does, because the original returns the number of elements it processed as a separate value
20:12:10shka_phoe: i didn't track the split sequence, but i can see how something conceptually simple can devolve into monstrosity
20:12:20phoefe[nl]ix: this is the first time I wrote a random fuzzer test
20:12:51sjlthe interaction between start/end, count, and remove-empty-subseqs is... nontrivial to get exactly the same as the original.
20:12:52shka_that's why i'm always for well structured code even for simple stuff
20:15:36shka_phoe: well, it is, extra arguments makes it more complicated
20:16:06phoeshka_: in other words: yes, you are correct, if we didn't need to conform to the original API and test suite, we'd gladly have made this code much nicer
20:19:24sjl(though performance was the entire reason phoe rewrote this in the first place, so really if we didn't care about performance we'd still have the original...)
20:19:55phoeoh right, the original split-sequence was quadratic on lists
20:21:53shka_sjl: i am simply recommending restructuring this a little bit further
20:22:11shka_it is really not that far from the ideal
20:22:26sjlshka_: feel free to PR it. If you can come up with something that performs as well and is clearer, I'm sure phoe would be willing to merge it