- Didn't allocate the arrays for the incoming and outgoing lines or didn't initialize after allocating.
Allocation and initialization are separate steps:
IncomingLine [] incoming = new IncomingLine[numin];
for (int i = 0; i < numin; i++)
incoming[i] = new Incoming(i, 0, arrival);
- Addressed the array from 1 to
numin rather
than from 0 to numin-1. (Java array indices start at 0.)
- Used a
while
loop and didn't update the index, making the loop infinite.
(Use a for loop to step through every element of an array.)
- Didn't correct the
clearBusy problem when a message was received so one an outgoing line
was busy, it was always busy.
- Always put the message on the
OutgoingLine whose ID was the same
as the ID of the IncomingLine of the sender.
- Assumed the number of incoming and outgoing lines were the same and only had a three parameter
constructor.
- Repeated identical code in both constructors. The case of one incoming and one outgoing line are
special cases of the new constructor. This constructor should be written:
public UnbufferedNodeModel(double arrival, double sendtime) {
this(arrival, sendtime, 1, 1);
}
The initialization code should not be repeated in two places.
- Allocated the arrays in the field declarations to be a fixed size:
private IncomingLines incoming = new IncomingLines[10];
private OutgoingLines outgoing = new OutgoingLines[10];
Never allocate arrays or objects in the fields declarations except in certain cases of static variables.
This approach limits the number of incoming and outgoing lines (e.g. to 10). The implementation then required that
extra counter variables be carried around to indicate how many array slots were actually being allocated. Another problem
has to do with unpredictable exception handling. DON'T DO THIS.
- Only ever sent a message to the last outgoing line in the array. (This problem was caused by
inappropriate bracing of the
for loop. The
loop just went through all of the elements each time.)
- Counted a lost message for every outgoing line that was busy, not just when all of the outgoing lines were busy.
- Generated a new MSG_RECEIVE event for every incoming line each time a message was sent. (A new event should only be
generated for the incoming line that sent the message.)
- Didn't update the
toString method to include the arrays. A good approach here would be to write a method that
could be called for both of the arrays:
private String getArrayString(String name, Object array[]) {
StringBuffer buf = new StringBuffer();
buf.append(name + "[");
for (int i = 0; i < array.length - 1; i++)
buf.append("[" + i + "]=" + array[i].toString() + ",");
if (array.length > 0)
buf.append("[" + (array.length-1) + "]=" + array[array.length-1].toString());
buf.append("]");
return buf.toString();
}
We use StringBuffer because concatenation using String recopies the array on each concatenation.