tags:

views:

763

answers:

7
package abc;

class DependencyDataCollection
{
    private int sNo;
    private String sessionID;
    private int noOfDependency;
    private int noOfRejection;
    private int totalValue;

    /** Creates a new instance of DependencyDataCollection */
    public DependencyDataCollection(int sNo, String sessionID, int noOfDependency, int noOfRejection, int totalValue)
    {
        this.sNo = sNo;
        this.sessionID = sessionID;
        this.noOfDependency = noOfDependency;
        this.noOfRejection = noOfRejection;
        this.totalValue = totalValue;
    }

    public int getSNo()
    {
        return sNo;
    }

    public String getSessionID()
    {
        return sessionID;
    }

    public int getNoOfDependency()
    {
        return noOfDependency;
    }

    public int getNoOfRejection()
    {
        return noOfRejection;
    }

    public int getTotalValue()
    {
        return totalValue;
    }
}

public class DependencyStack {

    LinkedList lList;

    /** Creates a new instance of DependencyStack */
    public DependencyStack()
    {
        lList = new LinkedList();
    }

    public void add(int sNo, String sessionID, int noOfDependency, int noOfRejection, int totalValue)
    {
        lList.add(new DependencyDataCollection(sNo,sessionID,noOfDependency,noOfRejection,totalValue));
    }

    public int size()
    {
        return lList.size();
    }

    public void show()
    {
        for(int i=0;i<lList.size();i++)
        {
            DependencyDataCollection ddc = (DependencyDataCollection)lList.get(i);
            System.out.println(ddc.getSNo()+"   "+ddc.getSessionID()+"   "+ddc.getNoOfDependency()+"     "+ddc.getNoOfRejection()+"      "+ddc.getTotalValue());
        }
    }

    public int returnIndexOfSession(String sessionID)
    {
        DependencyDataCollection ddc = null;
        for(int i=0;i<lList.size();i++)
        {
            ddc = (DependencyDataCollection)lList.get(i);
            if(ddc.getSessionID().equals(sessionID))
                break;
        }
        return ddc.getSNo();
    }

    public static void main(String args[])
    {
        DependencyStack ds = new DependencyStack();
        ds.add(1,"a",0,0,0);
        ds.add(2,"b",0,0,0);
        ds.show();

        //System.out.println(ds.returnIndexOfSession("a"));

//        DependencyDataCollection ddc = new DependencyDataCollection(1,"a",0,0,0);
//        System.out.println(ds.indexOf(ddc));
    }
}

This is a simple Linked List program in java which is using inbuilt linkedlist class from java.util package. The linked list is used to store different amount of data using DependencyDataCollection Class..

Now my question is

1) Please evaluate this program, and temme am i respecting all java concepts like private member access,which i have done, etc..

2) I am facing problem to find the indexOf a particular session.

For e.g. Node 1 contains 1,"a",0,0,0...............Node 2 contains 2,"b",0,0,0

Now i wanted to find the indexOf the node which is containing one of the data as "b" or "a". What could be the shortest inbuilt method which can do so, as i have made a function named "public int returnIndexOfSession(String sessionID)" which makes use of for loop, i find this very time consuming.. Is there any other way out..

Please evaluate and guide as i am a newbie in java.

+1  A: 

One thing that stands out is the lack of javadoc style comments (from http://en.wikipedia.org/wiki/Javadoc )

/**
 * Validates a chess move. Use {@link #doMove(int, int, int, int)} to move a piece.
 * 
 * @param theFromFile file from which a piece is being moved
 * @param theFromRank rank from which a piece is being moved
 * @param theToFile   file to which a piece is being moved
 * @param theToRank   rank to which a piece is being moved
 * @return            true if the chess move is valid, otherwise false
 */
SeanJA
Thanks for that answer,, really appreciable.. Will take this into notice frm now..
AGeek
+1  A: 

You want to use generics:

List<DependencyDataCollection> lList;

Also, in your variable definition you should use the interface List and not the concrete type (LinkedList).

For indexOf to work, your element type (DependencyDataCollection) needs to implement the comparator for equality:

class DependencyDataCollection{

  @Override
  public boolean equals(Object o){
    ...
  }
}

You can then use the built-in indexOf() that the List interface provides. It will however do the same kind of looping that you do now. If that is too time-consuming (really?) then you need a hash-backed list or something (in which case you would also need to implement hashCode()).

Update: It will do the same kind of looping, but more efficiently than you do now. Do not access a linked list by index it is not built for that, use the foreach loop or an Iterator (or an ArrayList):

    for(DependencyDataCollection d: iList){
    ... }
Thilo
A: 

In recent versions of Java, you can use generics so that you don't have to cast the object produced when you call lList.get(i). For example:

LinkedList<DependencyDataCollection> lList = new LinkedList<DependencyDataCollection>();

...

DependencyDataCollection ddc = lList.get(0);

To get the index of a specific element, loop through the list manually. For each index i in the range of the list, get the DependencyDataCollection there and see if it has the properties you want. If it does, save the index.

Nikhil Chelliah
Thanks,, its working..
AGeek
+1  A: 

YOu're off to a good start. Refinements you could make:

  • Generics. Your linked list could be

    LinkedList < DependencyDataCollection > lList; This gives you type checking.

  • You've seen here that your linked list is inconvenient to search - you have to check each value, which is slow and unwieldy. This is why people use hashmaps.

  • Look up the builder pattern to find a way around the long list of constructor args.

Steve B.
Can you give me some examples of using Hashmaps.. Some reference material or website...
AGeek
Also, your 3rd point was not clear to me..
AGeek
+1  A: 

1) Why don't you use Generics:

LinkedList<DependencyDataCollection> lList;

2) That is the downside of LinkedList, you might as well use HashMap or other data structures

bLee
Can you give me some reference website or the name of the datastructure which can neglect this drawback of linkedlist..
AGeek
@Kool Techie: HashMap is basically a data structure with key and value pair. You can map a key with a value, and you can get the value back with the key. I suggested HashMap because it seems as if you need to get a value using another value. More on HashMap: http://lmgtfy.com/?q=how+to+use+hashmap
bLee
+2  A: 

Here are the changes I would make, rationale is in the comments.

import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.Writer;
import java.util.ArrayList;
import java.util.List;


class DependencyDataCollection
{
    // makte them fnal, then you hava an immutible object and your code is much safer.
    // in your case you had noset methods so it was safe, but always try to make things final.
    private final int sNo;
    private final String sessionID;
    private final int noOfDependency;
    private final int noOfRejection;
    private final int totalValue;

    public DependencyDataCollection(final int    sNo, 
                                    final String sessionID, 
                                    final int    noOfDependency, 
                                    final int    noOfRejection, 
                                    final int    totalValue)
    {
        this.sNo            = sNo;
        this.sessionID      = sessionID;
        this.noOfDependency = noOfDependency;
        this.noOfRejection  = noOfRejection;
        this.totalValue     = totalValue;
    }

    public int getSNo()
    {
        return sNo;
    }

    public String getSessionID()
    {
        return sessionID;
    }

    public int getNoOfDependency()
    {
        return noOfDependency;
    }

    public int getNoOfRejection()
    {
        return noOfRejection;
    }

    public int getTotalValue()
    {
        return totalValue;
    }
}

class DependencyStack
{
    // change the type to be as generic as poosible - List interface
    // added generics so you get compile time safety and don't use casts later on
    // renamed it to something meaningful
    private final List<DependencyDataCollection> dependencies;

    // use an ArrayList instead of a LinkedList, it'll be faster since you are not inserting/deleting
    // into the middle of the list
    {
        dependencies = new ArrayList<DependencyDataCollection>();
    }

    // your Stack shouldn't know how to make the collections... (in my opinion)
    public void add(final DependencyDataCollection ddc)
    {
        dependencies.add(ddc);
    }

    public int size()
    {
        return dependencies.size();
    }

    // the next 3 methods are just convenience since you don't know if someione
    // will want to write to a file or a writer instead of a stream
    public void show()
    {
        show(System.out);
    }

    public void show(final OutputStream out)
    {
        show(new OutputStreamWriter(out));
    }

    public void show(final Writer writer)
    {
        show(new PrintWriter(writer));
    }

    public void show(final PrintWriter writer)
    {
        // use the new for-each instead of the old style for loop
        // this also uses an iterator which is faster than calling get
        // (well on an ArrayList it probably is about the same, but a LinkedList it'll be faster)
        for(final DependencyDataCollection ddc : dependencies)
        {
            writer.println(ddc.getSNo()            + "   " +
                           ddc.getSessionID()      + "   " +
                           ddc.getNoOfDependency() + "   " +
                           ddc.getNoOfRejection()  + "   " +
                           ddc.getTotalValue());
        }
    }

    public int returnIndexOfSession(final String sessionID)
    {
        DependencyDataCollection foundDDC;
        final int                retVal;

        foundDDC = null;

        for(final DependencyDataCollection ddc : dependencies)
        {
            if(ddc.getSessionID().equals(sessionID))
            {
                foundDDC = ddc;
                break;
            }
        }

        // deal with the fact that you might have not found the item and it would be null.
        // this assumes -1 is an invalid session id
        if(foundDDC == null)
        {
            retVal = -1;
        }
        else
        {
            retVal = foundDDC.getSNo();
        }

        return (retVal);
    }

    public static void main(final String[] args)
    {
        DependencyStack ds = new DependencyStack();
        ds.add(new DependencyDataCollection(1,"a",0,0,0));
        ds.add(new DependencyDataCollection(1,"a",0,0,0));
        ds.show();

        //System.out.println(ds.returnIndexOfSession("a"));

//        DependencyDataCollection ddc = new DependencyDataCollection(1,"a",0,0,0);
//        System.out.println(ds.indexOf(ddc));
    }
}

Edit:

This one will speed up the lookups (and deletes).

class DependencyStack
{
    // A Map provides quick lookup
    private final Map<String, DependencyDataCollection> dependencies;

    // a LinkedHashMap allows for quick lookup, but iterates in the order they were added... if that matters for show.
    {
        dependencies = new LinkedHashMap<String, DependencyDataCollection>();
    }

    // your Stack shouldn't know how to make the collections... (in my opinion)
    public void add(final DependencyDataCollection ddc)
    {
        if(ddc == null)
        {
            throw new IllegalArgumentException("ddc cannot be null");
        }

        dependencies.put(ddc.getSessionID(), ddc);
    }

    public int size()
    {
        return dependencies.size();
    }

    // the next 3 methods are just convenience since you don't know if someione
    // will want to write to a file or a writer instead of a stream
    public void show()
    {
        show(System.out);
    }

    public void show(final OutputStream out)
    {
        show(new OutputStreamWriter(out));
    }

    public void show(final Writer writer)
    {
        show(new PrintWriter(writer));
    }

    public void show(final PrintWriter writer)
    {
        // use the new for-each instead of the old style for loop
        // this also uses an iterator which is faster than calling get
        // (well on an ArrayList it probably is about the same, but a LinkedList it'll be faster)
        for(final DependencyDataCollection ddc : dependencies.values())
        {
            writer.println(ddc.getSNo()            + "   " +
                           ddc.getSessionID()      + "   " +
                           ddc.getNoOfDependency() + "   " +
                           ddc.getNoOfRejection()  + "   " +
                           ddc.getTotalValue());
        }
    }

    public int returnIndexOfSession(final String sessionID)
    {
        final DependencyDataCollection ddc;
        final int                      retVal;

        if(sessionID == null)
        {
            throw new IllegalArgumentException("sessionID cannot be null");
        }

        // get it if it exists, this is much faster then looping through a list
        ddc = dependencies.get(sessionID);

        // deal with the fact that you might have not found the item and it would be null.
        // this assumes -1 is an invalid session id
        if(ddc == null)
        {
            retVal = -1;
        }
        else
        {
            retVal = ddc.getSNo();
        }

        return (retVal);
    }

    public static void main(final String[] args)
    {
        DependencyStack ds = new DependencyStack();
        ds.add(new DependencyDataCollection(1,"a",0,0,0));
        ds.add(new DependencyDataCollection(1,"a",0,0,0));
        ds.show();

        //System.out.println(ds.returnIndexOfSession("a"));

//        DependencyDataCollection ddc = new DependencyDataCollection(1,"a",0,0,0);
//        System.out.println(ds.indexOf(ddc));
    }
}
TofuBeer
i have to delete a node in future,, thats why i have used linkedlist... comment..
AGeek
I have to change the values of sno,sessionID etc.. as the program progresses. So i think i cannot use final.. Please comment over this..
AGeek
Still, thanks a lot sir,, your editable program,, taught me lot new things.. thanx a lot...
AGeek
If you have to change them then yes, they cannot be final. If the sessionID changes they you cannot use the Map version. Also the Map one assumes that the equals and hashCode methods of the DependencyDataCollection are overrriden
TofuBeer
The map works great... Hands of.. We can also delete a node from this Map type declaration?? Also can you provide me the guide where we could get JavaDocs..
AGeek
hanks a lot sir, you rock!! With your edited programs, i gained a lot new things in java being a newbie.. thanks sir
AGeek
http://java.sun.com/javase/6/docs/api/java/util/LinkedHashMap.html You can delete items by calling remove. Make sure you override the equals and hashCode methods on the DependencyDataCollection or you won't be able to get/remove the objects.
TofuBeer
A: 

LinkedList is in java.util and can hold any type of class. For example I use a class called Application. So in the code bellow I just have a list of applications. Then i put one application in the list. I can also iterate over my applications to do whatever I want with my objects.

     LinkedList<Application> app = new LinkedList<Application>();

            app.add(new Application("firefox"));

 Iterator<Application> iterable = app.iterator();
 while(iterable.hasNext()){
  Application eachapp = iterable.next();
 }

If you need to find an object with indexOf you could override the equal method for your object in my case "Application" so if in equal I declared that and application its equal to and other app if the field name is the same then

app.indexOf(new Application("firefox")) will return the index of the app I just inserted.

JorgeO
Thanx sir, for the answer.. I have used LinkedHashMap class, and looks quite satisfied.. If it has any drawback over LinkedList, then plz comment..
AGeek
No, diferent java objects, a map is a set of entries wich contain keys and values. LinkedList its just a list of objects. With linkedlist you can do FIFO a LIFO queues stuff like that. With maps it more like a set of keys so you can find a value.
JorgeO