Network layer simulator - Implementation of the transport layer

I'm working on a programming assignment which simulates network traffic between two hosts. My task is to implement the transport layer, the code for the other layers, GUI etc. is already provided. I have managed to do this and the code works according to the specifications listed below. My concern is that the code feels badly structured and could be improved significantly both in regards of structure and functionality.

Video demonstration of a working implementation from our instructors.

Requirements:

• Uses the constructors values for timer and window size.

• Sends packets that arrive from the applicationlayer if there's space in the window.

• Buffers packets which arrives from the application layer if there's no space in the window.

• Resends all packets that has been sent but not ACK:ed in case of a timer interrupt.

• Responds to correct packets that arrive in order with an ACK.

• Responds to correct packets that has been previously received with an ACK.

• Ignores packets that are incorrect and/or arrives in the wrong order.

• Sends packets from the buffer if an ACK has been received and there's space in the window.

Below is my code for the transport layer. I'm not sure if the rest of the simulator's code (which is not written by me and should not be reviewed) is needed to do a review but I will include a link to it anyway.

TransportLayer.java:

```
package protocolsimulator;
import java.util.HashMap;

import protocolsimulator.Segment;

public class TransportLayer
{
private LayerSimulator mLayerSimulator = null;
private String mId;
private int mTimerValue, mWindowSize;
private int seqNumber = -1;
private int ackNumber = -1;
private int expected = 0;

private HashMap sentSegments = new HashMap();
private HashMap receivedACKS = new HashMap();
private HashMap recvBuffer = new HashMap();
private HashMap sendBuffer = new HashMap();

/**
* Consructs a TransportLayer
*
* @param id Id fo

This doesn't look right:

private int seqNumber = -1;
private int ackNumber = -1;
private int expected = 0;

I'd expect expected to be initialized with a value of 0, so the = 0 is redundant. The two -1 defaults are off-putting though: it makes it look like 0 would be a valid ackNumber/seqNumber, and my instinct if telling me "nope". If there's a reason to initialize these two at -1, a comment is in order.

public void sendACK(Segment s){

    Segment ack = new Segment();
    ack.seqNumber = -1;
    ack.from = "B";
    ack.ackNumber = s.seqNumber;
    ack.payload = "ACK " + s.payload.charAt(0);
    System.out.println( mId + " sent ACK: " + s.payload);
    mLayerSimulator.toNetworkLayer(ack);

}

Why hard-code the -1 here? Was the seqNumber field meant to be that default value? If so, the field should probably be static final... but it seems odd that a "sequence number" would be a constant, hard-coded or not.

Also, if I'm reading this right, you're reporting success before you actually send the acknowledgement. Either change "sent" to "sending", or move that System.out.println statement after the line that's actually doing the work. Otherwise you could be reporting success even when toNetworkLayer throws an exception, which seems likely given it's presumably accessing a network.

private void send(Segment s){

    System.out.println(mId + " sends " + s.toString());

    if(!sentSegments.containsValue(s)){
        sentSegments.put(s.seqNumber,s);
    }

    mLayerSimulator.toNetworkLayer(s);
    mLayerSimulator.startTimer(mTimerValue);
}

Again, "sends" is a confusing wording. I'd go with a "sending..." wording, and then you might have a problem when toNetworkLayer throws an exception and you've already added the Segment to your sentSegments. And if it's already in the sentSegments, is it normal and intended that you re-send it?

StackExchange Code Review Q#91981, answer score: 5