views:

873

answers:

9

My problem is actually more nuanced than the question suggests, but wanted to keep the header brief.

I have a HashMap<String, File> of File objects as values. The keys are String name fields which are part of the File instances. I need to iterate over the values in the HashMap and return them as a single String.

This is what I have currently:

private String getFiles()
{   
    Collection<File> fileCollection = files.values();
    StringBuilder allFilesString = new StringBuilder();

    for(File file : fileCollection) {
        allFilesString.append(file.toString());
    }
    return allFilesString.toString();
}

This does the job, but ideally I want the separate File values to be appended to the StringBuilder in order of int fileID, which is a field of the File class.

Hope I've made that clear enough.

+4  A: 

Unfortunately there is no way of getting data out of a HashMap in any recognizable order. You have to either put all the values into a TreeSet with a Comparator that uses the fileID, or put them into an ArrayList and sort them with Collections.sort, again with a Comparator that compares the way you want.

The TreeSet method doesn't work if there are any duplicates, and it may be overkill since you're not going to be adding things to or removing things from the Set. The Collections.sort method is a good solution for instances like this where you're going to take the whole HashSet, sort the results, and then toss away the sorted collection as soon as you've generated the result.

Paul Tomblin
Elements in a TreeSet are sorted by their keys, not their values...
pgras
A TreeSet doesn't have keys. You're thinking of TreeMap.
Paul Tomblin
A: 

Why not collect it in an array, sort it, then concatenate it?

-- MarkusQ

MarkusQ
A: 

You'll have to add your values() Collection to an ArrayList and sort it using Collections.sort() with a custom Comparator instance before iterating over it.

BTW, note that it's pointless to initialize the StringBuffer with the size of the collection, since you'll be adding far more than 1 character per collection element.

Michael Borgwardt
A: 

Create a temporary List, then add each pair of data into it. Sort it with Collections.sort() according to your custom comparator then you will have the List in your desired order.

Here is the method you're looking for: http://java.sun.com/javase/6/docs/api/java/util/Collections.html#sort(java.util.List,%20java.util.Comparator)

Edison Gustavo Muenz
+6  A: 

Something like this should work:

List<File> fileCollection = new ArrayList<File>(files.values());

Collections.sort(fileCollection, 
                 new Comparator<File>() 
                 {
                     public int compare(File fileA, File fileB) 
                     {
                         final int retVal;

                         if(fileA.fileID > fileB.fileID)
                         {
                             retVal = 1;
                         }
                         else if(fileA.fileID < fileB.fileID)
                         {
                             retVal = -1;
                         }
                         else
                         {
                             retVal = 0;
                         }

                         return (retVal);                         
                     }
                 });
TofuBeer
Sort that integer overflow out!
Tom Hawtin - tackline
Thanks for that, I've implemented the changes you suggested, see below.
milkplusvellocet
return fileA.fileID - fileB.fileID;
Adrian
Integer.MIN_VALUE - 1 = Integer.MAX_VALUE etc...
TofuBeer
A: 

I created LinkedHashMap a dozen times before it was added to the set of collections.

What you probably want to do is create a TreeHashMap collection.

Creating a second collection and appending anything added to both isn't really a size hit, and you get the performance of both (with the cost of a little bit of time when you add).

Doing it as a new collection helps your code stay clean and neat. The collection class should just be a few lines long, and should just replace your existing hashmap...

If you get in the habit of always wrapping your collections, this stuff just works, you never even think of it.

Bill K
A: 
StringBuffer allFilesString = new StringBuffer(fileCollection.size());

Unless all your file.toString() is one character on average, you are probably making the StringBuffer too small. (If it not right, you may as well not set it and make the code simpler) You may get better results if you make it some multiple of the size. Additionally StringBuffer is synchronized, but StringBuilder is not and there for more efficient here.

Peter Lawrey
Thanks Peter, I've replaced my StringBuffer with StringBuilder and removed the parameter from the constructor.
milkplusvellocet
A: 

OK, this is what I've come up with. Seems to solve the problem, returns a String with the File objects nicely ordered by their fileId.

public String getFiles()
{   
    List<File> fileList = new ArrayList<File>(files.values());

    Collections.sort(fileList, new Comparator<File>()
                               {
                                   public int compare(File fileA, File fileB)
                                   {
                                       if(fileA.getFileId() > fileB.getFileId()) 
                                       {
                                           return 1;
                                       }
                                       else if(fileA.getFileId() < fileB.getFileId()) 
                                       {
                                           return -1;
                                       }
                                       return 0;
                                   }
                               });

    StringBuilder allFilesString = new StringBuilder();

    for(File file : fileList) {
        allFilesString.append(file.toString());
    }
    return allFilesString.toString();
}

I've never used Comparator before (relatively new to Java), so would appreciate any feedback if I've implemented anything incorrectly.

milkplusvellocet
A: 

Remove unnecessary if statment.

List<File> fileCollection = new ArrayList<File>(files.values());
Collections.sort(fileCollection, 
             new Comparator<File>() {
                 public int compare(File a, File b) {
                     return (a.fileID - b.fileID);
                 }
             });
Adrian