Skip to content

Commit 99cab92

Browse files
committed
Refactor Reactor::{connect, connectMulti, canConnect}
0. `canConnect` doesn't experience situations where throwing Error is required. 1. `canConnect` is solely for testing and should not throw. 2. `connect` and `connectMulti` should throw. Simply return result from `canConnect` and throw in functions that calls them.
1 parent b81e1ce commit 99cab92

File tree

1 file changed

+79
-23
lines changed

1 file changed

+79
-23
lines changed

src/core/reactor.ts

Lines changed: 79 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,51 @@ export interface Call<A, R> extends Write<A>, Read<R> {
6161
invoke: (args: A) => R | undefined;
6262
}
6363

64+
export enum CanConnectResult {
65+
SUCCESS = 0,
66+
SELF_LOOP,
67+
DESTINATION_OCCUPIED,
68+
DOWNSTREAM_WRITE_CONFLICT,
69+
NOT_IN_SCOPE,
70+
RT_CONNECTION_OUTSIDE_CONTAINER,
71+
RT_DIRECT_FEED_THROUGH,
72+
RT_CYCLE,
73+
MUTATION_CAUSALITY_LOOP
74+
}
75+
76+
const CanConnectResultReasonMap = new Map<CanConnectResult, string>([
77+
[CanConnectResult.SUCCESS, "Connection successful."],
78+
[
79+
CanConnectResult.SELF_LOOP,
80+
"Source port and destination port are the same."
81+
],
82+
[
83+
CanConnectResult.DESTINATION_OCCUPIED,
84+
"Destination port is already occupied."
85+
],
86+
[
87+
CanConnectResult.DOWNSTREAM_WRITE_CONFLICT,
88+
"Destination port is already occupied."
89+
],
90+
[
91+
CanConnectResult.NOT_IN_SCOPE,
92+
"Source and destination ports are not in scope."
93+
],
94+
[
95+
CanConnectResult.RT_CONNECTION_OUTSIDE_CONTAINER,
96+
"New connection is outside of container."
97+
],
98+
[
99+
CanConnectResult.RT_DIRECT_FEED_THROUGH,
100+
"New connection introduces direct feed through."
101+
],
102+
[CanConnectResult.RT_CYCLE, "New connection introduces cycle."],
103+
[
104+
CanConnectResult.MUTATION_CAUSALITY_LOOP,
105+
"New connection will change the causal effect of the mutation that triggered this connection."
106+
]
107+
]);
108+
64109
/**
65110
* Abstract class for a schedulable action. It is intended as a wrapper for a
66111
* regular action. In addition to a get method, it also has a schedule method
@@ -1091,18 +1136,21 @@ export abstract class Reactor extends Component {
10911136
* @param src The start point of a new connection.
10921137
* @param dst The end point of a new connection.
10931138
*/
1094-
public canConnect<R, S extends R>(src: IOPort<S>, dst: IOPort<R>): boolean {
1139+
public canConnect<R, S extends R>(
1140+
src: IOPort<S>,
1141+
dst: IOPort<R>
1142+
): CanConnectResult {
10951143
// Immediate rule out trivial self loops.
10961144
if (src === dst) {
1097-
throw Error("Source port and destination port are the same.");
1145+
return CanConnectResult.SELF_LOOP;
10981146
}
10991147

11001148
// Check the race condition
11011149
// - between reactors and reactions (NOTE: check also needs to happen
11021150
// in addReaction)
11031151
const deps = this._dependencyGraph.getUpstreamNeighbors(dst); // FIXME this will change with multiplex ports
11041152
if (deps !== undefined && deps.size > 0) {
1105-
throw Error("Destination port is already occupied.");
1153+
return CanConnectResult.DESTINATION_OCCUPIED;
11061154
}
11071155

11081156
if (!this._runtime.isRunning()) {
@@ -1114,10 +1162,13 @@ export abstract class Reactor extends Component {
11141162
// Rule out write conflicts.
11151163
// - (between reactors)
11161164
if (this._dependencyGraph.getDownstreamNeighbors(dst).size > 0) {
1117-
return false;
1165+
return CanConnectResult.DOWNSTREAM_WRITE_CONFLICT;
11181166
}
11191167

1120-
return this._isInScope(src, dst);
1168+
if (!this._isInScope(src, dst)) {
1169+
return CanConnectResult.NOT_IN_SCOPE;
1170+
}
1171+
return CanConnectResult.SUCCESS;
11211172
} else {
11221173
// Attempt to make a connection while executing.
11231174
// Check the local dependency graph to figure out whether this change
@@ -1131,7 +1182,7 @@ export abstract class Reactor extends Component {
11311182
src._isContainedBy(this) &&
11321183
dst._isContainedBy(this)
11331184
) {
1134-
throw Error("New connection is outside of container.");
1185+
return CanConnectResult.RT_CONNECTION_OUTSIDE_CONTAINER;
11351186
}
11361187

11371188
// Take the local graph and merge in all the causality interfaces
@@ -1148,23 +1199,21 @@ export abstract class Reactor extends Component {
11481199

11491200
// 1) check for loops
11501201
const hasCycle = graph.hasCycle();
1202+
if (hasCycle) {
1203+
return CanConnectResult.RT_CYCLE;
1204+
}
11511205

11521206
// 2) check for direct feed through.
11531207
// FIXME: This doesn't handle while direct feed thorugh cases.
1154-
let hasDirectFeedThrough = false;
1155-
if (src instanceof InPort && dst instanceof OutPort) {
1156-
hasDirectFeedThrough = dst.getContainer() === src.getContainer();
1157-
}
1158-
// Throw error cases
1159-
if (hasDirectFeedThrough && hasCycle) {
1160-
throw Error("New connection introduces direct feed through and cycle.");
1161-
} else if (hasCycle) {
1162-
throw Error("New connection introduces cycle.");
1163-
} else if (hasDirectFeedThrough) {
1164-
throw Error("New connection introduces direct feed through.");
1208+
if (
1209+
src instanceof InPort &&
1210+
dst instanceof OutPort &&
1211+
dst.getContainer() === src.getContainer()
1212+
) {
1213+
return CanConnectResult.RT_DIRECT_FEED_THROUGH;
11651214
}
11661215

1167-
return true;
1216+
return CanConnectResult.SUCCESS;
11681217
}
11691218
}
11701219

@@ -1258,11 +1307,17 @@ export abstract class Reactor extends Component {
12581307
if (dst === undefined || dst === null) {
12591308
throw new Error("Cannot connect unspecified destination");
12601309
}
1261-
if (this.canConnect(src, dst)) {
1262-
this._uncheckedConnect(src, dst);
1263-
} else {
1264-
throw new Error(`ERROR connecting ${src} to ${dst}`);
1310+
const canConnectResult = this.canConnect(src, dst);
1311+
// I know, this looks a bit weird. But
1312+
if (canConnectResult !== CanConnectResult.SUCCESS) {
1313+
throw new Error(
1314+
`ERROR connecting ${src} to ${dst}. Reason is ${
1315+
CanConnectResultReasonMap.get(canConnectResult) ??
1316+
CanConnectResult[canConnectResult]
1317+
}`
1318+
);
12651319
}
1320+
this._uncheckedConnect(src, dst);
12661321
}
12671322

12681323
protected _connectMulti<R, S extends R>(
@@ -1316,7 +1371,8 @@ export abstract class Reactor extends Component {
13161371
}
13171372

13181373
for (let i = 0; i < leftPorts.length && i < rightPorts.length; i++) {
1319-
if (!this.canConnect(leftPorts[i], rightPorts[i])) {
1374+
const canConnectResult = this.canConnect(leftPorts[i], rightPorts[i]);
1375+
if (canConnectResult !== CanConnectResult.SUCCESS) {
13201376
throw new Error(
13211377
`ERROR connecting ${leftPorts[i]}
13221378
to ${rightPorts[i]}

0 commit comments

Comments
 (0)