6 Replies - 300 Views - Last Post: 04 May 2013 - 12:33 PM Rate Topic: -----

#1 psh  Icon User is offline

  • D.I.C Head

Reputation: -3
  • View blog
  • Posts: 65
  • Joined: 25-March 13

is this really bad code?

Posted 04 May 2013 - 07:30 AM

im new to this stuff and i need some help...is the below code really bad? its a thread that handles different socket ports ..i know its bad programming could u suggest me any changes?




public class ClientHandler implements Runnable {
    
    private int port;
    
    public ClientHandler(int port) {
        this.port = port;
        
    }
    
    @Override
    public void run() {
        

        Socket client ;
        try
        {
            client = new Socket("localhost",port);
            System.out.println("Client started");
           
            OutputStream out = client.getOutputStream();
            PrintWriter writer = new PrintWriter(out);
            
            InputStream in = client.getInputStream();
            BufferedReader reader = new BufferedReader(new InputStreamReader(in));

            writer.write("hello.."+ "\n");
            writer.flush();
            
            String s = null;

            while(( s = reader.readLine()) != null)
            {             
                System.out.println("Server send you back:" + s);   
                break;
            }
            reader.close();
            writer.close();    
            client.close();
            
        }
        catch(IOException e)
        {
            run();
            System.out.println("try again");    
        }  
      }   
}




Is This A Good Question/Topic? 0
  • +

Replies To: is this really bad code?

#2 CasiOo  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 1391
  • View blog
  • Posts: 3,077
  • Joined: 05-April 11

Re: is this really bad code?

Posted 04 May 2013 - 09:04 AM

Don't just call run() again if you get an exception
You could try and make it reconnect, but then at least give it a max tries

Move some of your code into a couple of methods, or even add new classes to help yourself
	//Returns true if it connects
	private boolean connect(String host, int port) {
		try {
			client = new Socket(host, port);
			return true;
		}
		catch (IOException e) {
			return false;
		}
	}
	
	//Returns true if the streams are setup
	private boolean setupIO(Socket socket) {
		try {
            writer = new PrintWriter(client.getOutputStream());
            reader = new BufferedReader(new InputStreamReader(client.getInputStream()));
			return true;
		}
		catch (IOException e) {
			return false;
		}
	}
	
	private void sendMessage(String message) throws IOException {
		writer.write(message);
		writer.write("\n");
		writer.flush();
	}


Was This Post Helpful? 1
  • +
  • -

#3 psh  Icon User is offline

  • D.I.C Head

Reputation: -3
  • View blog
  • Posts: 65
  • Joined: 25-March 13

Re: is this really bad code?

Posted 04 May 2013 - 10:30 AM

View PostCasiOo, on 04 May 2013 - 04:04 PM, said:

Don't just call run() again if you get an exception
You could try and make it reconnect, but then at least give it a max tries

Move some of your code into a couple of methods, or even add new classes to help yourself
	//Returns true if it connects
	private boolean connect(String host, int port) {
		try {
			client = new Socket(host, port);
			return true;
		}
		catch (IOException e) {
			return false;
		}
	}
	
	//Returns true if the streams are setup
	private boolean setupIO(Socket socket) {
		try {
            writer = new PrintWriter(client.getOutputStream());
            reader = new BufferedReader(new InputStreamReader(client.getInputStream()));
			return true;
		}
		catch (IOException e) {
			return false;
		}
	}
	
	private void sendMessage(String message) throws IOException {
		writer.write(message);
		writer.write("\n");
		writer.flush();
	}



thanks for your time :sigh:
Was This Post Helpful? 0
  • +
  • -

#4 psh  Icon User is offline

  • D.I.C Head

Reputation: -3
  • View blog
  • Posts: 65
  • Joined: 25-March 13

Re: is this really bad code?

Posted 04 May 2013 - 11:30 AM

View PostCasiOo, on 04 May 2013 - 04:04 PM, said:

Don't just call run() again if you get an exception
You could try and make it reconnect, but then at least give it a max tries

Move some of your code into a couple of methods, or even add new classes to help yourself
	//Returns true if it connects
	private boolean connect(String host, int port) {
		try {
			client = new Socket(host, port);
			return true;
		}
		catch (IOException e) {
			return false;
		}
	}
	
	//Returns true if the streams are setup
	private boolean setupIO(Socket socket) {
		try {
            writer = new PrintWriter(client.getOutputStream());
            reader = new BufferedReader(new InputStreamReader(client.getInputStream()));
			return true;
		}
		catch (IOException e) {
			return false;
		}
	}
	
	private void sendMessage(String message) throws IOException {
		writer.write(message);
		writer.write("\n");
		writer.flush();
	}



how i can break in a catch block ? i tried to make a local variable counter and increase it after it tries to connect again..but i cant use a break in if loop..


catch(Exception e)
{
    //not allows a break in if loop without for or switch
    if(counter > 22 ){
     break;
    }
}



Was This Post Helpful? 0
  • +
  • -

#5 psh  Icon User is offline

  • D.I.C Head

Reputation: -3
  • View blog
  • Posts: 65
  • Joined: 25-March 13

Re: is this really bad code?

Posted 04 May 2013 - 11:42 AM

View PostCasiOo, on 04 May 2013 - 04:04 PM, said:

Don't just call run() again if you get an exception
You could try and make it reconnect, but then at least give it a max tries

Move some of your code into a couple of methods, or even add new classes to help yourself
	//Returns true if it connects
	private boolean connect(String host, int port) {
		try {
			client = new Socket(host, port);
			return true;
		}
		catch (IOException e) {
			return false;
		}
	}
	
	//Returns true if the streams are setup
	private boolean setupIO(Socket socket) {
		try {
            writer = new PrintWriter(client.getOutputStream());
            reader = new BufferedReader(new InputStreamReader(client.getInputStream()));
			return true;
		}
		catch (IOException e) {
			return false;
		}
	}
	
	private void sendMessage(String message) throws IOException {
		writer.write(message);
		writer.write("\n");
		writer.flush();
	}



is this a good way to check for reconnection ?i have a field counter which i increase everytime method run is called..



  catch(Exception e)
        {
            reconnect();
        }

        }
    
    public void reconnect(){  
        
        switch(counter){
            
                case 15:
                    System.out.println("failed to connect");
                    break;
                default:
                    run();   
    }





Was This Post Helpful? 0
  • +
  • -

#6 CasiOo  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 1391
  • View blog
  • Posts: 3,077
  • Joined: 05-April 11

Re: is this really bad code?

Posted 04 May 2013 - 12:16 PM

I have no idea why you added a switch with the case 15 :D
private boolean reconnect(String host, int port, int tries) {
	final int timeout = 2000; //2 sec
	
	while (tries > 0) {
		if (connect(host, port, timeout))
			System.out.println("Reconnected!");
			return true;
		else {
			tries--;
			System.out.println("Failed to reconnect. " + tries + " tries left.");
		}
	}
	
	System.out.println("Failed to reconnect.");
	return false;
}


This post has been edited by CasiOo: 04 May 2013 - 12:17 PM

Was This Post Helpful? 1
  • +
  • -

#7 psh  Icon User is offline

  • D.I.C Head

Reputation: -3
  • View blog
  • Posts: 65
  • Joined: 25-March 13

Re: is this really bad code?

Posted 04 May 2013 - 12:33 PM

View PostCasiOo, on 04 May 2013 - 07:16 PM, said:

I have no idea why you added a switch with the case 15 :D/>
private boolean reconnect(String host, int port, int tries) {
	final int timeout = 2000; //2 sec
	
	while (tries > 0) {
		if (connect(host, port, timeout))
			System.out.println("Reconnected!");
			return true;
		else {
			tries--;
			System.out.println("Failed to reconnect. " + tries + " tries left.");
		}
	}
	
	System.out.println("Failed to reconnect.");
	return false;
}



because i have a variable counter which i increase everytime run is called...but thanks :blush:

Was This Post Helpful? 0
  • +
  • -

Page 1 of 1