WIP: Align stack size with long size and add generators

[vilu]

Just opening this up for some feedback, will continue to clean up and push to this.

[vilu]

I've added generators (I'll clean up later) however, seems like the large push/pop test broke. Will have to check that implementation too.

[vilu]

Ok, didn't know that. Thanks. I'll clean up the code when I got everything working and I'll make sure things are only calculated when necessary. Den lör 25 maj 2019 kl 14:41 skrev Regis Kuckaertz <notifications@github.com

…

: ***@***.**** commented on this pull request. ------------------------------ In core/shared/src/main/scala/scalaz/zio/internal/StackBool.scala <#898 (comment)>: > @@ -27,30 +27,32 @@ private[zio] final class StackBool private () { private[this] var _size = 0L final def getOrElse(index: Int, b: Boolean): Boolean = { - var j = index.toLong + val i0 = _size & 63L + val i = ((64L - i0) + index) & 63L + var ib = ((64L - i0) + index) / 64L I think bitwise shift is much faster than division. You can as well avoid computing (64L - i0) + index twice 😃 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#898>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALLTV43N4AKHZVN4YKAPPLPXEXXPANCNFSM4HPUKU4A> .

[vilu]

I kept this part because it was there before. In theory it should never blow up.

…

Sent from my iPhone

On 25. May 2019, at 18:06, Roman Tkalenko ***@***.***> wrote: @tkroman commented on this pull request. In core/shared/src/main/scala/scalaz/zio/internal/StackBool.scala: > cur = cur.next } - assert(j < 256L && j >= 0) + assert(i < 64L && i >= 0) isn't i always going to be < 64? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[tkroman]

yes, i mean there is no need for assert on this because i is immutable, there was an assert before on j because it changes in the loop, so maybe you want to change to assert on the value of ie (the variable one)?

[vilu]

I ended up removing the assertion. Thanks for pointing it out.

[LGLO]

Thank you! Generally looks very good. I think one null check is missing in while in getOrElse.
For "laws" I like ScalaCheck, but we still can have simple checks for some scenarios, like getting element with too high index.

[vilu]

@LGLO, @regiskuckaertz thanks for your feedback. I've tried to address your comments.

[vilu]

For "laws" I like ScalaCheck, but we still can have simple checks for some scenarios, like getting element with too high index.

@LGLO Can you think of any other case besides too high index (I've added one check there)?

[LGLO]

IDK if there are any laws that bind stack and list, but my intuition is that toList should yield same result (besides mutation) like pop-ing all elements. However non-mutating toList is just inefficient and I consider this as "debug" method only.

[vilu]

Yes, I was thinking about that too. I had a quick look around and I was seeing different behaviours which made me think that it should just continue the way it behaved before.

import java.util.Stack;
import java.util.ArrayList;

public class Main {
    public static void main(String args[]) {
        Stack<Boolean> stack = new Stack<Boolean>();
        stack.push(true);
        stack.push(true);
        stack.push(false);
        stack.push(true);
        stack.push(false);
        ArrayList<Boolean> list = new ArrayList(stack);
        for (int i = 0; i < list.size(); i++) {
          System.out.println(list.get(i));
        }
    }
}

// true
// true
// false
// true
// false
---------------------------------
scala> res27
res29: s.type = Stack(false, true, false, true, true)

scala> res27.toList
res30: List[Boolean] = List(false, true, false, true, true)
---------------------------------
val s = StackBool()

s.push(true)
s.push(true)
s.push(false)
s.push(true)
s.push(false)


println(s.toList)
//List(false, true, false, true, true)

[LGLO]

@vilu From my side it looks OK! I don't merge right now because I'd like to take another look but I don't have time today. I'll take a look tomorrow.

[jdegoes]

@vilu Thank you!