bwmarrin / snowflake

A simple to use Go (golang) package to generate or parse Twitter snowflake IDs
BSD 2-Clause "Simplified" License
2.98k stars 371 forks source link

Why do we need the nodeMax? #46

Closed CarsonSlovoka closed 1 year ago

CarsonSlovoka commented 2 years ago

nodeMax seems to be generated only to determine whether the given node is reasonable, so should we consider removing it from the structure?

bwmarrin commented 1 year ago

So are you saying that instead of using nodeMax value we could just do the calculation at the time we do the check that the node size is valid? Or are you thinking something else?

Does having the nodeMax value cause some sort of issue?

CarsonSlovoka commented 1 year ago

Does having the nodeMax value cause some sort of issue?

nodeMax works well.


I just thought about whether to let the structure become simple

When I see a structure with many attributes, I will need to read many things, reducing my desire to read further. 😅

    n := Node{}
    n.node = node
    // n.nodeMax = -1 ^ (-1 << NodeBits)
    nodeMax := -1 ^ (-1 << NodeBits)
    n.nodeMask = n.nodeMax << StepBits
    n.stepMask = -1 ^ (-1 << StepBits)
    n.timeShift = NodeBits + StepBits
    n.nodeShift = StepBits

    // if n.node < 0 || n.node > n.nodeMax {
    if n.node < 0 || n.node > nodeMax {
        return nil, errors.New("Node number must be between 0 and " + strconv.FormatInt(n.nodeMax, 10))
    }

Yes, this opinion is subjective

bwmarrin commented 1 year ago

Not a bad idea actually :) I like simple too. We could move this into a local var in the function and keep the struct smaller.