Skip to content

Comments

More on generated routing#183

Merged
alexander-yevsyukov merged 31 commits intomasterfrom
more-on-generated-routing
Feb 26, 2025
Merged

More on generated routing#183
alexander-yevsyukov merged 31 commits intomasterfrom
more-on-generated-routing

Conversation

@alexander-yevsyukov
Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov commented Feb 24, 2025

This PR continues work on generating routing setup classes.

Changes in details

  • StateRoutingSetup was implemented.
  • The missing documentation for the code introduced in Generated event routing, updated dependencies #181 was added.
  • StateUdateRouteVisitor and EventRouteVisitor got an abstract base MulticastRouteVisitor because the code generated inside the run block is basically the same with the only difference with the parameter types.
  • Added the check for route functions declared in an entity class that accept the same message type. If more than one function is discovered those functions are reported via Logger.error().

@alexander-yevsyukov alexander-yevsyukov self-assigned this Feb 24, 2025
@alexander-yevsyukov alexander-yevsyukov marked this pull request as ready for review February 26, 2025 14:58
Comment on lines 69 to 71
lamp = context.readState(l1)
// The command was handled via custom routing declared in the class.
lamp.state shouldBe State.ON
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
lamp = context.readState(l1)
// The command was handled via custom routing declared in the class.
lamp.state shouldBe State.ON
// The command was handled via custom routing declared in the class.
lamp = context.readState(l1)
lamp.state shouldBe State.ON

I would put these comments above the block, so that there is an empty before a comment. Then, it is easier to read them.

fun route(e: RoomEvent): RoomId = e.room

/**
* The routing function by event class.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* The routing function by event class.
* The routing function accepting the event class.

So that the both method are documented in the same style.

super.setupEventRouting(routing)
@Assign
internal fun handle(c: AddDevice): DeviceAdded =
deviceAdded { device = c.device; name = c.name }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why to have them on the same line? The methods below don't do that.

Copy link
Contributor Author

@alexander-yevsyukov alexander-yevsyukov Feb 26, 2025

Choose a reason for hiding this comment

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

Because these functions generate events, while appliers update the state.

Also, I'm experimenting with the compact style for code which merely transforms a command into an event without doing much work.

Comment on lines 36 to 37
* @param F The type of route functions handled by the visitor.
* @param setup The type of the routing setup class generated by this visitor.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please have an empty line before the constructor parameters.

Comment on lines 56 to 57
* The base class for code generators implementing routing setup classes
* adding calls to the type of route functions specified by the generic parameter [F].
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence is hard to understand.

Comment on lines 148 to 149
// Not duplicated.
it shouldNotContain "DeviceRegistered"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Not duplicated.
it shouldNotContain "DeviceRegistered"
// This event should not be duplicated.
it shouldNotContain "DeviceRegistered"

@alexander-yevsyukov alexander-yevsyukov merged commit 230fa3f into master Feb 26, 2025
7 checks passed
@alexander-yevsyukov alexander-yevsyukov deleted the more-on-generated-routing branch February 26, 2025 19:11
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.

3 participants