Kevik
Kevik

Reputation: 9351

Instantiating variables inside of separate thread that were declared on main thread, good or bad?

notice the code shown here where I am declaring variables like BufferedOutputStream in the MsgAndPack class, and later inside of a separate thread, after public void run() i am instantiating those same variables.

is this bad practice or is there any hidden dangers of doing this?

should i put both both of these things in the same place either inside or outside of the newly created thread?

 public class MsgAndPack implements Runnable {

         BufferedInputStream bistr;
         BufferedOutputStream bostr;
         FileInputStream fistr;
         FileOutputStream fostr;
         DataOutputStream dostr;
         long length;

    @Override
    public void run() {

        if (socket.isConnected()) {

            try {
                file = new File("/mnt/sdcard/JIend.db");
                length = file.length();
                byte[] bytes = new byte[(int) length];

                fistr = new FileInputStream(file);
                bistr = new BufferedInputStream(fistr);
                bostr = new BufferedOutputStream(socket.getOutputStream());
                dostr = new DataOutputStream(bostr); 
                int count;

Upvotes: 0

Views: 69

Answers (1)

Piotr Kołaczkowski
Piotr Kołaczkowski

Reputation: 2629

First, there is no such thing like "thread that the variables were declared on". Instantiation and access matters, not declaration place - there is no code for just the declaration.

If you are instantiating those variables inside run and use them only in run, why not declare them inside run, too? That'd be the simpliest and safest solution.

Declaring them at class level suggests you'll be accessing them from more than one method. If you do not need it, then it is confusing for whoever reads the code.

Additionally, you declared them as package-local, which suggests something outside of this class can access them. This is unsafe - it means they can be shared by multiple threads and this would cause a lot of trouble, because streams are stateful and not thread-safe.

So to summarize - it is not wrong per-se to declare those streams at class level as long as you initialize and reference them in only one thread, but it is very fragile. Initializing in one thread and accessing in another would require at least making them volatile (the other thread might not get the value update). So always use the smallest scope possible.

If your intent is sharing some fields among threads make sure access to them is properly synchronized (synchronized, volatile) or they themselves are thread-safe.

Upvotes: 3

Related Questions