Why ForEachKey when we have iterators?

[dr-orlovsky]

Sorry for potentially dumb question, but why do we need ForEachKey if we already have iterators? I checked trait implementations, they seem to replicate the code from iterators — and iterators should be more idiomatic than custom trait...

[apoelstra]

I did consider implementing this as a new iterator (it actually does not match the existing iterators exactly because it combines Pk and PkH but does not also include hashlocks or timelocks) and then letting users do .for_each on this.

My reasons were

• the implementation was a fair bit simpler and also more efficient
• it's a bit of a PITA to make iterators generic, although to be honest I didn't try very hard ... you'd have to do something like adding type PkIter; type PkhIter; type TimelockIter; etc to the DescriptorTrait trait, and then every implementor needs to create an explicit type for this (they can't use impl Iterator because Rust is annoying)

The efficiency thing I'm not too worried about ... I have some plans to overhaul our internal representation of trees to improve cache efficiency and iteration logic anyway ... but the other issues, I believe, are why the existing iterators are implemented for Miniscript but not any of the descriptor or policy types.

I'm open to the argument that we should just bite the bullet and write all this code, in the interest of being idiomatic.

[dr-orlovsky]

Yeah, I see the reasoning... Will try to think and play whether there can be anything done in practice with iterators, or we will be limited by a exponentially growing complexity

[stevenroose]

Yeah I agree that a iter_keys() method that returns an impl Iterator<Item=&Pk> would be great. I gave it a quick shot, but decided it involved a bit more thinking so I'm just gonna create HashSets using translate_pks (for_each_key doesn't guarantee that it passes each key because it stops as soon as the predicate returns false, so that's a bit subtle there, the name breaks with the Iterator::for_each, it's more an Iterator::all implementation).