Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sentinel-11: linearizability comment thru #11

Merged
merged 1 commit into from
Dec 21, 2023
Merged

Conversation

ReshiAdavan
Copy link
Owner

None

@ReshiAdavan ReshiAdavan added the Review PR is in review label Dec 21, 2023
func bitsetIndex(pos uint) (uint, uint) {
return pos / 64, pos % 64
}

// set sets the bit at the specified position to 1.
func (b bitset) set(pos uint) bitset {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning an error if the pos is out of range instead of silently failing.

func (b bitset) set(pos uint) bitset {
major, minor := bitsetIndex(pos)
b[major] |= (1 << minor)
return b
}

// clear sets the bit at the specified position to 0.
func (b bitset) clear(pos uint) bitset {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning an error if the pos is out of range instead of silently failing.

func (b bitset) clear(pos uint) bitset {
major, minor := bitsetIndex(pos)
b[major] &^= (1 << minor)
return b
}

// get returns true if the bit at the specified position is 1.
func (b bitset) get(pos uint) bool {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning an error if the pos is out of range instead of silently failing.

// data layout:
// bits 0-63 are in data[0], the next are in data[1], etc.

// newBitset creates a new bitset with a specified number of bits.
func newBitset(bits uint) bitset {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming the bits parameter to numBits or bitCount for clarity.

if bits%64 != 0 {
extra = 1
}
// Calculate total number of 64-bit chunks needed.
chunks := bits/64 + extra

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using math.Ceil function to calculate the number of chunks. This would make the code more readable and eliminate the need for the extra variable.

Step func(state interface{}, input interface{}, output interface{}) (bool, interface{})
// Equality on states.

// Equal function defines equality for states.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming the Equal function to AreStatesEqual for better readability and understanding.

Equal func(state1, state2 interface{}) bool
}

// NoPartition is a default partitioning function that treats the entire history as a single partition.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming the NoPartition function to SinglePartition for better readability and understanding.

func NoPartition(history []Operation) [][]Operation {
return [][]Operation{history}
}

// NoPartitionEvent is a default partitioning function for event histories, treating the entire history as a single partition.
func NoPartitionEvent(history []Event) [][]Event {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming the NoPartitionEvent function to SinglePartitionEvent for better readability and understanding.

func NoPartitionEvent(history []Event) [][]Event {
return [][]Event{history}
}

// ShallowEqual is a default equality function that checks for basic equality between two states.
func ShallowEqual(state1, state2 interface{}) bool {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming the ShallowEqual function to AreStatesShallowEqual for better readability and understanding.

return true, st + inp.Value
}
// Default case: should not happen in correct usage
return false, state

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good to have a default case in the switch statement, but it would be better to handle it properly. Instead of just returning false, state, consider throwing an error or panic to indicate that an invalid operation type was provided.

@ReshiAdavan ReshiAdavan merged commit d3b3d3c into master Dec 21, 2023
1 check passed
@ReshiAdavan ReshiAdavan deleted the Sentinel-11 branch December 21, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review PR is in review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant