Suggestion about type parameter naming in functions defined in Flow class

Akka
1

As I read through Flow class defined in Akka Stream code,
I felt type parameter declarations in the following functions a little bit confusing.

  def join[I2, O2, Mat2](bidi: Graph[BidiShape[Out, O2, I2, In], Mat2]): Flow[I2, O2, Mat] = joinMat(bidi)(Keep.left)

  def joinMat[I2, O2, Mat2, M](bidi: Graph[BidiShape[Out, O2, I2, In], Mat2])(combine: (Mat, Mat2) ⇒ M): Flow[I2, O2, M] = {

What I thought was strange is about type parameter name O2.

In BidiShape class definition, the type parameter naming below is adopted.

final case class BidiShape[-In1, +Out1, -In2, +Out2]

Therefore, when we express BidiShape as a drawing, that would be like this :

        +------+
  In1 ~>|      |~> Out1
        | bidi |
 Out2 <~|      |<~ In2
        +------+

But, in join and joinMat definitions, type parameter naming seems to not correspond to
that of BidiShape.

Out1 in BidiShape is translated into O2 in join and joinMat definitions.

As far as my understanding of these two functions, this Flow is connected to BidiFlow like the following figure.

 +---------------------------------+
 | Resulting Flow                  |
 |                                 |
 | +------+              +------+  |
 | |      | ~Out(= I1)~> |      | ~~> O1
 | | flow |              | bidi |  |
 | |      | <~In(= O2)~  |      | <~~ I2
 | +------+              +------+  |
 +---------------------------------+

So, I would suggest to change names of type parameter O2 in join and joinMat
to O1 as declared in BidiShape.

Of course I understand there are many things I may misunderstand about Akka code,
So I would like to hear opinions about this suggestion by Akka experts.

If this suggestion seems reasonable, I’m ready to send PR for it.

2

Out and In are type parameters from the Flow instance you are doing a join on, they are not type parameters from the Flow.join method, you cannot rename those for one particular use case (with BidiShape) as they need to fit all the operations on a flow.

3

Thanks for your answer. You’re right.
I totally agree with Out and In are type parameters from the Flow instance and I cannot rename just for join and joinMat.
What I would like to question here is about O2 defined only in join and joinMat. We cannot rename this parameter, either?

The thing is, when I first read these methods declaration, I felt O2 corresponded to Out2 in BidiShape, although this is because of my poor understanding.
I thought O2 is just a type parameter name defined only for each method, and I felt using O1 like below would give impression that the output of join's resulting flow is Out1 in BidiShape.

  def join[I2, O1, Mat2](bidi: Graph[BidiShape[Out, O1, I2, In], Mat2]): Flow[I2, O1, Mat] = joinMat(bidi)(Keep.left)

  def joinMat[I2, O1, Mat2, M](bidi: Graph[BidiShape[Out, O1, I2, In], Mat2])(combine: (Mat, Mat2) ⇒ M): Flow[I2, O1, M] = {
4

Ah, sorry, now I see what you are getting at, yes you are right, O2 should probably be O1 given that it corresponds to out 1 on the bidishape. Please PR if you want!

5

Thank you for taking time and considering it :smile:
I just created PR#26828 for it. I would like you to review it when you’re available.

1 Like
6

Hi :)
I noticed I didn’t received enough approval to be merged for PR#26828.
Should I ask for review explicitly for other members?

7

@heraklos Thanks for the reminder. I looked at it now and left one comment, then we can merge. Sorry for delay in reviewing and thanks for contributing.

1 Like