While loop/sockets/timer complications question....

[Olliepop]

Hey guys :)

I am stuck on this piece of code i am about to give below. What happens, is the server sends a "ping" to the client, the client sends "pong" back, but when the server sends "pong" back to the client (i KNOW this is sent) the client doesn't pick it up.

dataIn is a thread started on the client.

Are there any errors you guys pick up here?
This thread is started from an applet,

The part where "pong" should be picked up by the client is

} else if(in.readLine().equalsIgnoreCase("pong")) {
						t1.stop();
						System.out.println("Latency: " + PingTime + "ms.");
						PingTime = 0;
					}

This is the full class code:

public class dataIn implements Runnable, ActionListener {
	static BufferedReader in;
	Boolean threadcontinue = true;
	static String o;
	static String o1;
	Socket s;
	Timer t1 = new Timer(10, this);
	public static int PingTime = 0;
	
	dataIn(Socket s) {
		this.s = s;
	}

	public void run() {
		// TODO Auto-generated method stub
		try {
			in = new BufferedReader(new InputStreamReader(s.getInputStream()));
			try {
				while(threadcontinue==true) {
					if(in.readLine().equalsIgnoreCase("ping")==true) {
						PrintStream pstream2 = new PrintStream(s.getOutputStream());
				      	pstream2.println("pong");
				      	t1.start();
						System.out.println("pong sent!!");
					} else if(in.readLine().equalsIgnoreCase("pong")) {
						t1.stop();
						System.out.println("Latency: " + PingTime + "ms.");
						PingTime = 0;
					}
					
				}
			} finally {
				
			}
		} catch (Throwable t) {
				// TODO Auto-generated catch block
				t.printStackTrace();
			} 
			
	}

	@Override
	public void actionPerformed(ActionEvent e2) {
		// TODO Auto-generated method stub
		if (e2.getSource() == t1) {
			PingTime = PingTime + 10;
			System.out.println(PingTime);
			if(PingTime > 100) {
				t1.stop();
			}
		}
	}
}

I would appreciate so much any insight possible, or even a solution. Thank you very much for your time!

[JamesCherrill]

You stop() before you deal with the pong. Using stop() is a bad idea. It's deprecated. Set your threadcontinue var to false to exit the run cleanly.
Anyway - if the first readline isn't ping then you do a second readline to check for pong. That's one readline to many, and the cause of your problem.
Finally
while(threadcontinue==true) {
is redundant, just use
while(threadcontinue) {

[Olliepop]

Anyway - if the first readline isn't ping then you do a second readline to check for pong. That's one readline to many, and the cause of your problem.

Hey, thanks for your reply.

What do you suggest i change to do what you said above - use 1 readline?

[JamesCherrill]

while(threadcontinue) {
   String input = in.readLine().toLowerCase();
   if(input.equals("ping")) {
      ...
   } 
   else if (input.equals("pong")) { 
      threadcontinue = false;
      etc

[Olliepop]

Ok nevermind, i got it! For those who may have a similar issue, here is the source code of the file fixed, which is working completely! Thanks James, you were right about the readlines.

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintStream;
import java.net.Socket;

import javax.swing.Timer;


public class dataIn implements Runnable, ActionListener {
	static BufferedReader in;
	static String ex1;
	Boolean threadcontinue = true;
	static String o;
	static String o1;
	Socket s;
	Timer t1 = new Timer(10, this);
	public static int PingTime = 0;
	
	dataIn(Socket s) {
		this.s = s;
	}

	public void run() {
		// TODO Auto-generated method stub
		try {
			in = new BufferedReader(new InputStreamReader(s.getInputStream()));
			
				System.out.println("dataIn started.");
				while(threadcontinue) {
					in = new BufferedReader(new InputStreamReader(s.getInputStream()));
					ex1 = in.readLine();
					if(ex1.equalsIgnoreCase("ping")==true) {
						System.out.println("received ping");
						PrintStream pstream2 = new PrintStream(s.getOutputStream());
				      	pstream2.println("pong");
				      	t1.start();
						System.out.println("pong sent!!");
					} else if(ex1.equalsIgnoreCase("pong")) {
						System.out.println("received pong");
						System.out.println("Latency: " + PingTime + "ms.");
						PingTime = 0;
						t1.stop();
					}
					
				}
			
		} catch (Throwable t) {
				// TODO Auto-generated catch block
				t.printStackTrace();
			} 
			
	}

	@Override
	public void actionPerformed(ActionEvent e2) {
		// TODO Auto-generated method stub
		if (e2.getSource() == t1) {
			PingTime = PingTime + 10;
			if(PingTime > 100) {
				System.out.println("t1 stopped");
				t1.stop();
			}
		}
	}
}

Edit: just saw your reply when i posted this :P thanks for the example, i understood a couple of minutes after asking

[JamesCherrill]

That's good. Time to mark this as "solved".

[JamesCherrill]

ps: I see you still have the stop() method. It's a really bad idea - you'll get unexplainable thread problems unless you remove it. Also still saying ==true - a personal pet hate of mine.

[Olliepop]

ps: I see you still have the stop() method. It's a really bad idea - you'll get unexplainable thread problems unless you remove it. Also still saying ==true - a personal pet hate of mine.

Ok i'll change those lol - what do you suggest i do instead of .stop?

[JamesCherrill]

Ok i'll change those lol - what do you suggest i do instead of .stop?

See my very first post. (Just set threadcontinue to false)