Skip to content

Add Nand functionality #110

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

Merged
merged 2 commits into from
Aug 18, 2015
Merged

Add Nand functionality #110

merged 2 commits into from
Aug 18, 2015

Conversation

evanh
Copy link
Contributor

@evanh evanh commented Aug 5, 2015

One useful use case for bitarrays is to do filtering of values,
by removing the values in one bitarry from another. Add a Nand
function that will do this sort of filtering.

This is a use case that is necessary for my application, which is
doing high performance filtering.

I know that the And and Nand functionality is very similar, but it seemed
different enough to warrant a separate file. Let me know what you think.

One useful use case for bitarrays is to do filtering of values,
by removing the values in one bitarry from another. Add a Nand
function that will do this sort of filtering.
// Here, our indices match for both `sba` and `other`.
// Time to do the bitwise AND operation and add a block
// to our result list if the block has values in it.
resultBlock = sba.blocks[selfIndex].nand(other.blocks[otherIndex])
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not like this duplication. Far too fragile.

Might I suggest a style such as:

type blockOperation func(Block, Block) Block

func and(Block, Block) Block { ... }
func nand(Block, Block) Block { ... }

func compareSparseWithSparseBitArray(operation blockOperation, sba, other *sparseBitArray) BitArray {
    // ...
    resultBlock = operation(sba.blocks[selfIndex], other.blocks[otherIndex])
    // ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I started going down that road at first, but I found that the functions differed beyond just the block comparison. So I tried a way where I passed flags in to do decisioning but then the code got even messier.

As well, I realized that I actually needed to add a new function specifically for nand, to handle the extra case of dense nand sparse. With and, you can handle the dense and sparse case with a single function, since it's commutative, but nand isn't.

The binary Read function, while clean, was causing large performance
hits by requiring a bytes Reader. Speed this up by removing the reader
and parsing the bytes directly.
@dustinhiatt-wf
Copy link
Contributor

+1

1 similar comment
@alexandercampbell-wk
Copy link
Contributor

+1

dustinhiatt-wf added a commit that referenced this pull request Aug 18, 2015
@dustinhiatt-wf dustinhiatt-wf merged commit 3b07818 into Workiva:master Aug 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants