Skip to content

Conversation

@laviRZ
Copy link

@laviRZ laviRZ commented Jan 8, 2026

No description provided.

@laviRZ laviRZ added the question Further information is requested label Jan 8, 2026
Comment on lines +116 to +119
final BaseStatusSignal[] newSignals = new BaseStatusSignal[RIO_SIGNALS.length + 1];
System.arraycopy(RIO_SIGNALS, 0, newSignals, 0, RIO_SIGNALS.length);
newSignals[RIO_SIGNALS.length] = statusSignal;
RIO_SIGNALS = newSignals;
Copy link
Author

Choose a reason for hiding this comment

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

Why not make RIO_SIGNALS an ArrayList so it has a dynamic size, and wherever needed convert it to a regular array?
If performance matters, it's possible to cache the result of the conversion

Copy link
Member

Choose a reason for hiding this comment

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

Not sure I fully understand you. Caching seems more wasteful at runtime


@Override
public void toLog(LogTable table) {
if (numberOfInputs == 0 && signalToThreadedQueue.isEmpty())
Copy link
Author

Choose a reason for hiding this comment

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

What's this check?

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean?

}

private void updateSignalsToTable(LogTable table) {
if (firstInputIndex == -1 || numberOfInputs == 0)
Copy link
Author

Choose a reason for hiding this comment

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

it's enough to check for firstInputIndex == -1, because it has to be true when numberOfInputs == 0

Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not to check both? Seems safer/more scaleable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants