For now just used for index/findIndex, but there are more uses for walkBucketChain
Diff Detail
- Repository
- R33 KDevPlatform
- Lint
Lint Skipped - Unit
Unit Tests Skipped
serialization/itemrepository.h | ||
---|---|---|
1354–1357 | replace [&] with [this, request]. and as I say below, I find it odd that the return value decides both whether the loop is stopped, and the return value of the walkBucketChain method. | |
1833 | some documentation please. and I find the choice of return types a bit odd. the lambda should always return true/false, no? and that would then tell the loop to break or not. the walkBucketChain could then return void. The return value of the lambda could be given out via a captured variable? |
serialization/itemrepository.h | ||
---|---|---|
1833 | I do understand what you're saying by return types being odd, but having walkBucketChain return void or bool is not really a readability improvement in any situation. We don't want void, since that means we /require/ a sentinel value to tell us if we broke early or found nothing, which won't necessarily be useful otherwise. Ie, in deleteItem, if found we set one value, else another one, so we'd either have to initialize it to zero and then set it again if found, or add another bool to tell us that it was found. We could just have walkBucketChain return, but aside from just being noisier, it seems like premature pessimization. I think for this case where we are always working with pointers or zero-invalid bucket indexes, it makes some sense to have it as it is. Of course if you still find it undesirable, I'll change it. |
minor nitpicks, I'm in favor of this. How do I give a "ship it" on phabricator?
serialization/itemrepository.h | ||
---|---|---|
1825 | put brace on newline |