CODEWITHSUNDEEP
javaandroidperformancefor-loopgarbage-collection
Ederbit
Ederbit

Reputation: 205

Avoid creating multiple Class Objects in a for loop- Performance

Suppose I have a class named Song with lots of information like

A setter in this class sets the Song title. 

public class Song {
    private String title;
    private String artist;
    ...

    public Song(String songTitle, String songArtist, ...) {
        this.title=songTitle;
        this.artist=songArtist;
        ...
    }

    public void setTitle(String Songname){
        this.title = Songname;
    }

    public String getTitle(){return title;}
    public String getArtist(){return artist;}
    ...
}

I have a List of Strings with different song titles, but no other information about the songs. Now I want to populate a List of Songs and just set the title of those Songs classes with the help of the setter to the titles in the List Strings. I do this in a for-statement, but if I check the List entries I only get the title of the last Song that was added.

int index = 0;
Song songClass = new LocalSong(null, null, ...);
for (final String song : Songs) {
    try {
        songClass.setTitle(song);
        //Set Titles for Local Song Objects
        AnotherList.add(index, null);
        AnotherList.set(index, songClass);
        System.out.println("Title of Song number " + index + " is " + AnotherList.get(index).getTitle());
        index++;
    } catch (Exception e) {e.printStackTrace();}
}

I found that when I don't use the setter in the Song class and create a new Song object inside the for-statement for each List of Strings entry, it works. Like this:

int index = 0;
for (final String songname : Songs) {
    try {
        Song songClass = new LocalSong(songname, null, ...);
        //Set Titles for Local Song Objects
        AnotherList.add(index, null);
        AnotherList.set(index, songClass);
        System.out.println("Title of Song number " + index + " is " + AnotherList.get(index).getTitle());
        index++;
    } catch (Exception e) {e.printStackTrace();}
}

But isn't that solution performance-heavy? For instance if I would have hundreds of entries in the List of Strings, i would create hundreds of song classes that i don't use, or does the garbage collector handle this well?

Also, is there a better way to create Songclasses with Titles that I get from a List of Strings.

Upvotes: 1

Views: 1705

Answers (5)

Pratik Bhatt
Pratik Bhatt

Reputation: 1

Easiest way to restricting multiple instance of

import java.awt.event.WindowEvent;
import java.awt.event.WindowListener;
import java.io.*;

import javax.swing.JFrame;


public class SingleWindow {
    static String str;
    
    private SingleWindow() {
        
    }
    
    private static void single() {
        readFile();
        System.out.println("Single str =" + str);
        if (str.equals("false")) {
            gui();
            writeFile("true");
        }
    }
        
    private static void writeFile(String str){
        try {
            PrintWriter pw = new PrintWriter("E:\\Temp.txt");
            pw.print(str);
            pw.close();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }   
    }
    
    static void readFile() {
        String status = null;
        System.out.println("readFile() entered \n");
        try {
            FileReader fr = new FileReader("E:\\temp.txt");
            BufferedReader br = new BufferedReader(fr);
            while((status = br.readLine())!=null){
                str = status;
                System.out.println("Status " + status);
            }
            br.close();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
    
    static void gui() {
        System.out.println("Entered GUI method \n");
        JFrame frame = new JFrame();
        frame.setVisible(true);
        frame.setSize(300,300);
        frame.addWindowListener(new WindowListener() {
            @Override
            public void windowOpened(WindowEvent arg0) {}
            
            @Override
            public void windowIconified(WindowEvent arg0) {}
            
            @Override
            public void windowDeiconified(WindowEvent arg0) {}
            
            @Override
            public void windowDeactivated(WindowEvent arg0) {}
            
            @Override
            public void windowClosing(WindowEvent arg0) {
                System.out.println("Windows Closing \n");
                writeFile("false");
            }
            
            @Override
            public void windowClosed(WindowEvent arg0) {
                System.out.println("Windows Closed \n");
            }
            
            @Override
            public void windowActivated(WindowEvent arg0) {
                //writeFile("false");
                System.out.println("Windows Activate \n");
            }
        });
    }
    
    
    public static void main(String args[]) {
        single();
        System.out.println(str + "main method \n");
    }
}

Upvotes: 0

Alan Macdonald
Alan Macdonald

Reputation: 1900

You seem to have a fundamental misunderstanding of the keyword new which creates new instances of objects. If you only new an object once then all object references in the list are pointing at the same object, therefore it's the last setter that dictates that values seen no matter which reference is accessed.

You are also attempting to do what is called "premature optimisation". Be wary of this as you will often over-complicate and hence adversely impact maintainability of your code by focusing on this. It's ok to do minor easy optimisations or choose alternatives when they are just as easy or easy to refactor to, but be careful you're not solving a non existent problem.

Upvotes: 2

marcinj
marcinj

Reputation: 49986

There is nothing unusual here, in first example you reuse the same object, your list contains the same references to the same single object. Since you modify it you are modifying also the whole list.

But isn't that solution performance-heavy?

no, at least until you will measure that it takes too much memory or your loop execution takes too much time. If that happen then you should redesign your code, ie. process your song list in chunks, only what user sees on screen. Maybe redesign your UI, allow your user to see songs alphabetically - only on latter A, B, C, ... - whatever suits your app.

Upvotes: 1

kevchoi
kevchoi

Reputation: 434

The reason why the second solution works is because you're creating a NEW instance of a Song class every iteration of the for loop and adding it to the list.

In the first solution, you're adding the SAME instance every iteration of the for loop, songClass. What you're doing here is changing the title of songClass, and adding it to the list.

As for garbage collection, it's perfectly fine at this stage of the code to create a list with a lot of objects. The garbage collector will deallocate objects when you aren't using them anymore.

Upvotes: 1

chrylis -cautiouslyoptimistic-
chrylis -cautiouslyoptimistic-

Reputation: 77177

You're referring to instances of the Song class, not lots of classes, and object-oriented programming is all about creating instances whenever you have identifiably distinct objects. A single HTTP request in a modern Java framework will typically create and then discard thousands of objects, and the JVM is engineered to handle it just fine.

More broadly, don't optimize for performance unless you have a demonstrated performance problem--it's actually slow, and investigation (such as profiling) tells you where the problem is.

Upvotes: 3

Related Questions