File reader and streamer component running in its own thread on Android

In an Android project,
I have a class whose job is to read lines from a file,
and then pump those lines to a message handling thread,
looping forever until told to stop.
The main operation of this class (file reading + message sending) must be on its own separate thread,
to run in parallel with whatever else the application is doing.
Multi-threaded programming is not my strongest suit,
so I'd like to reach out to those more knowledgeable than me in this area.

```
package net.bluetoothviewer;

import android.content.res.AssetManager;

import net.bluetoothviewer.util.AssetUtils;

import java.util.List;

public class MockLineByLineConnector implements DeviceConnector {

public static final String SAMPLES_SUBDIR = "samples/line-by-line";

private static final int SLEEP_MILLIS = 1000;

private final MessageHandler messageHandler;
private final AssetManager assets;
private final String filename;

private boolean running = false;

public MockLineByLineConnector(MessageHandler messageHandler, AssetManager assets, String filename) {
this.messageHandler = messageHandler;
this.assets = assets;
this.filename = filename;
}

@Override
public synchronized void connect() {
if (running) {
return;
}
running = true;
new Thread(new Runnable() {
@Override
public void run() {
messageHandler.sendConnectingTo(filename);

String mockFilePath = SAMPLES_SUBDIR + "/" + filename;
List lines = AssetUtils.readLinesFromStream(assets, mockFilePath);

if (!lines.isEmpty()) {
loopLinesUntilStopped(lines);
}

messageHandler.sendConnectionLost();
}

private void loopLinesUntilStopped(List lines) {
messageHandler.sendConnectedTo(filename);

while (running) {
for (String line : lines)

One synchronise to much :

@Override
public synchronized void disconnect() {
    running = false;
}

You are declaring this synchronised. So if 2 threads are calling this method, the second one has to wait until the first one is finished.

Because you don't have an input variable, the output will always be the same, setting running false.
In this case there are no race conditions what could affect the outcome of this method, so I wouldn't make it synchronised.

private static final :

You are making a great use of the private static final fields.

What I'm not liking is the next step :

String mockFilePath = SAMPLES_SUBDIR + "/" + filename;

Why don't you put the "/" also in the SAMPLES_SUBDIR?

It's the only place where you use this field so it could be easily in the SAMPLES_SUBDIR.

Comments :

While I'm not the first to say you have to document your code, the following case is for me strange :

@Override
public void sendAsciiMessage(CharSequence chars) {
    // do nothing
}

You can do 2 things here.

• Throwing a UnsupportedOperationException so the next developer knows, this method you need to override is not supported here.

• Putting more comment then do nothing, why doesn't this has to do nothing? When I read that in a project that I have to take over, I'm thinking maybe TODO but why didn't he put TODO? So I'll be searching for the why and loosing maybe valuable time over a stupid issue.

All over :

Nice readable code, as an outstander, who doesn't know the things the other classes need to do I still understand what this class has to do.

StackExchange Code Review Q#79904, answer score: 2