Path: csiph.com!usenet.pasdenom.info!weretis.net!feeder4.news.weretis.net!eternal-september.org!feeder.eternal-september.org!mx04.eternal-september.org!.POSTED!not-for-mail From: Daniele Futtorovic Newsgroups: comp.lang.java.programmer Subject: Re: Java (android) socket reconnection Date: Sun, 09 Dec 2012 19:07:36 +0100 Organization: A noiseless patient Spider Lines: 216 Message-ID: References: <9a8716eb-5842-45d8-b62e-193122cd863e@googlegroups.com> <27f309f0-5118-4fa8-8b26-858d19af27de@googlegroups.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Injection-Date: Sun, 9 Dec 2012 18:07:59 +0000 (UTC) Injection-Info: mx04.eternal-september.org; posting-host="471490eb7dbb5112d18d2869517c5aa4"; logging-data="675"; mail-complaints-to="abuse@eternal-september.org"; posting-account="U2FsdGVkX19KvFkeDFvzmM8arJK/6GSy" User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:12.0) Gecko/20120428 Thunderbird/12.0.1 In-Reply-To: <27f309f0-5118-4fa8-8b26-858d19af27de@googlegroups.com> Cancel-Lock: sha1:x209u73Ywl96FYrR9eZI4FUfvVA= Xref: csiph.com comp.lang.java.programmer:20196 On 09/12/2012 17:26, artik allegedly wrote: > My whole code looks like. I have to use to threads due to manage obtaining data from server too. Could You look at it in free time and tell me what's wrong? > > Artik, Below you'll find a bunch of code relating to your issues. It is not intended to be usable as such, but rather to give you some ideas about how you /could/ organise things, as well as addressing some of the problems your original code has, notably, as pointed out by Eric, the synchronisation issues. You tried to manage those with timers, but that's hardly ever a good idea. A couple of points: 1. Don't use tabs in source code. Configure your IDE to convert tabs to spaces. Four spaces per indent level is the standard. 2. Avoid mixing GUI code and back-end code. Build a model, and then use that model in the GUI code. 3. *ENCAPSULATE*! In your code, you shared the underlying objects (the socket, the writer), then built structures on top of them (the Runnables), and tried to orchestrate those structures via manipulation of the underlying objects. That's the wrong way to go. Rather, you should define layers of complexity: the bottom-most, then the first, second, third, etc., layer. A piece of layer N should only ever interact with other pieces of layer N, or with pieces of layer N-1, but never further than that -- never with N-2. It's N-1's responsibility to interact with N-2. I don't know if that's clear enough, but it's the best explanation I can come up for now. And it's very important if you want to build clean software. 4. When catching an InterruptedException, always, always call Thread.currentThread().interrupt() in the catch block. 5. As a side-note, whenever transmitting text, beware of charset encodings. I've highlighted this in the code example by specify the UTF-8 charset. Unless you have a server that can't talk UTF-8, this is what you should always do. Code: ------------------------------------------------------------------- package scratch; import java.io.BufferedWriter; import java.io.Closeable; import java.io.IOException; import java.io.OutputStreamWriter; import java.io.Writer; import java.net.InetSocketAddress; import java.net.Socket; import java.nio.channels.SocketChannel; public class ConnectionsExample { public static interface Connector extends Closeable { Writer getWriter() throws IOException; void reset(); } public static class Poller implements Runnable { private final Connector connector ; private final long sleepTime ; public Poller(Connector connector, long sleepTime) { this.connector = connector; this.sleepTime = sleepTime; } @Override public void run() { System.err.println("Entering poller"); for( ; ! Thread.currentThread().isInterrupted(); ){ try { Writer w = connector.getWriter(); w.write("TEST DATA\n"); w.flush(); } catch ( IOException ioex ){ System.err.println("Polling failed: "); ioex.printStackTrace(); connector.reset(); } try { Thread.sleep(sleepTime); } catch ( InterruptedException iex ){ Thread.currentThread().interrupt(); } } System.err.println("Exiting poller"); try { connector.close(); } catch ( IOException ioex ){ System.err.println("Could not close connector"); ioex.printStackTrace(); } } } public static Connector createConnector( final String address, final int port ){ return new Connector(){ private Socket socket; private Writer writer; private boolean disposed = false; @Override public synchronized Writer getWriter() throws IOException { if( disposed ){ throw new IOException( "Connector is closed" ); } if( socket != null && ! socket.isClosed() ){ assert writer != null; return writer; } else if( ! socket.isClosed() ){ try { socket.close(); } catch ( IOException ioex ){ System.err.println("Could not close old socket:"); ioex.printStackTrace(); } } SocketChannel sc; try { System.err.printf( "Opening connection to: %s:%d%n", address, port ); sc = SocketChannel.open( new InetSocketAddress(address, port) ); } catch ( IOException ioex ){ throw new IOException( String.format("Could not open connection to %s:%d. Will retry later.", address, port), ioex ); } this.socket = sc.socket(); this.socket.setSoLinger(true, 1); this.socket.setTcpNoDelay(true); this.writer = new BufferedWriter( new OutputStreamWriter(this.socket.getOutputStream(), "UTF-8") //XXX mind the charset!! ); return this.writer; } private synchronized void closeSocket( boolean dispose ) throws IOException { try { if( socket != null ){ socket.close(); } } finally { if( dispose ) this.disposed = true; } } @Override public void reset() { try { closeSocket( false ); } catch ( IOException ioex ){ System.err.println("Could not reset connector:"); ioex.printStackTrace(); } } @Override public void close() throws IOException { closeSocket( true ); } }; } public static Runnable createPoller( String address, int port, long pollDelay ){ Connector c = createConnector( address, port ); return new Poller( c, pollDelay ); } public static void createAndStartPoller( String address, int port, long pollDelay ){ Runnable poller = createPoller( address, port, pollDelay ); Thread t = new Thread( poller ); t.setDaemon( true ); t.start(); } } -------------------------------------------------------------------