Skip to content

Conversation

antiguru
Copy link
Member

This removes a bunch of now redundant code.

This removes a bunch of now redundant code.

Signed-off-by: Moritz Hoffmann <[email protected]>
Copy link
Member

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

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

I'm good with this, though also the pub type things tripped me up when I was trying to use an alias for DistributorPact. Like, it was hard to use because you can't use an alias everywhere you can use a type. I don't think I use Exchange or ExchangeCore that often though, but .. we might find out that this is irritating. But I think even in that case, a new struct that just wraps things to present as a new type would be fine, and still a simplification. No rush to do it, until we need it, though.


/// An exchange between multiple observers by data
pub struct ExchangeCore<CB, F> { hash_func: F, phantom: PhantomData<CB> }
pub type ExchangeCore<CB, F> = DistributorPact<Box<dyn FnOnce(usize) -> DrainContainerDistributor<CB, F>>>;
Copy link
Member

Choose a reason for hiding this comment

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

A thing I'm uncertain about, but maybe it's fine, is that when I did pub type Foo = DistributorPact; I didn't have the ability to use Foo as a constructor, like let x = Foo(bar);. I'm not entirely certain when type aliases can be used and cannot, and we might find out that this alias results in making it harder to use ExchangeCore if you need to invoke DistributorPact instead. Maybe it's fine though, and I'm inventing problems that don't exist.

@frankmcsherry frankmcsherry merged commit e42ac3d into TimelyDataflow:master Sep 16, 2025
7 checks passed
@antiguru antiguru deleted the exchange_distribute branch September 16, 2025 18:06
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.

2 participants