MartinHH commented on code in PR #1787: URL: https://github.com/apache/pekko/pull/1787#discussion_r1984550609
########## stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Switch.scala: ########## @@ -0,0 +1,119 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.pekko.stream.impl.fusing + +import org.apache.pekko +import pekko.annotation.InternalApi +import pekko.stream.Attributes +import pekko.stream.FlowShape +import pekko.stream.Graph +import pekko.stream.Inlet +import pekko.stream.Outlet +import pekko.stream.SourceShape +import pekko.stream.impl.Stages.DefaultAttributes +import pekko.stream.scaladsl.Source +import pekko.stream.stage.GraphStage +import pekko.stream.stage.GraphStageLogic +import pekko.stream.stage.InHandler +import pekko.stream.stage.OutHandler + +/** + * INTERNAL API + */ +@InternalApi private[pekko] final class Switch[T, M] Review Comment: I'd be fine with renaming this, but maybe it's worth to explain my choice of name before we decide whether to rename: - This `GraphStage` does not evaluate the lambda - similar to `flatMapMerge` and `flatMapConcat`, the evalation of `f` is delegated to the `map` operator and then only the "flattening" is done in the `GraphStage`. (My reasons for implementing it that way: consistency with the afforementioned operators and less redundancy in code because I did not need to care about things like `f` throwing.) - For those other operators that delegate evaluation of `f` to `map`, the naming scheme seems to be to not have `Map` in the name of the `GraphStage` (the stage for `flatMapMerge` is called `FlattenMerge`) - The transformation that is implemented by this `GraphStage` ("switching" without the "mapping" part) corresponds to what an operator that is named `switch` in both ReactiveX and Monix does So following my train of thought, if this stage were named `SwitchMap` one should move the evaluation of `f` into the `GraphStage`. Don't get me wrong: I'd be fine with any solution (keep it as it is; rename to `SwitchMap` and move evaluation of `f` into the `GraphStage` or just rename and keep evaluation of `f` where it is), I just wanted to share my reasoning before I make any changes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org For additional commands, e-mail: notifications-h...@pekko.apache.org
