How do I rewrite this in a cleaner way?

Discussion in 'Java' started by Fencer, Jul 29, 2010.

  1. Fencer

    Fencer Guest

    Hello, I have a large set of strings and I need to find all combinations
    of two of those string and perform an algorithm on them (the algorithm
    is actually an XQuery that runs over a large dataset so I won't post
    that here). The combinations foo-bar and bar-foo are considered to be
    equal so only one should be generated.

    I wrote a static class method that takes an array of Strings and returns
    all combinations in a Vector. The Vector stores a class Pair which I've
    written myself too. The complete code is posted below with output from a
    sample run. It seems to work, but how can it be improved and get rid of
    the Pair class? Thanks!

    package utility;

    import java.util.HashSet;
    import java.util.Set;
    import java.util.Vector;

    public class MiscUtils {

    public static void main(String[] args) {
    Vector<Pair> combos = findCombinations(new String[]{"chebi",
    "kegg.compound"});
    System.out.println(combos.size());
    System.out.println(combos);
    }

    public static Vector<Pair> findCombinations(String[] strings) {
    Vector<Pair> combinations = new Vector<Pair>();
    Set<String> alreadyPaired = new HashSet<String>();

    for (int i = 0; i < strings.length; ++i) {
    for (int j = 0; j < strings.length; ++j) {
    if (i != j && !alreadyPaired.contains(strings[j])) {
    combinations.add(new Pair(strings, strings[j]));
    }
    }

    alreadyPaired.add(strings);
    }

    return combinations;
    }
    }

    class Pair {
    public Pair(String s1, String s2) {
    this.s1 = s1;
    this.s2 = s2;
    }

    public String getFirst() {
    return s1;
    }

    public String getSecond() {
    return s2;
    }

    public String toString() {
    return s1 + ":" + s2;
    }

    private String s1 = null;
    private String s2 = null;
    }

    Sample run:
    1
    [chebi:kegg.compound]

    - Fencer
     
    Fencer, Jul 29, 2010
    #1
    1. Advertisements

  2. Fencer

    Fencer Guest

    Thanks for your reply my Duniho. I should have been clearer: I iterate
    through all my combinations and runs the all algorithm on each
    combination. The algorithm is a complicated XQuery that runs over a
    large data set. The number of combinations I have is not that large
    actually, I just didn't want to do the pairing by hand. It's true that I
    don't really need to keep the combinations around but I would actually
    like to do that.
    The input data could actually contain duplicates but that would be an
    error on my part. I mostly want to get rid of the Pair class. :)
     
    Fencer, Jul 29, 2010
    #2
    1. Advertisements

  3. Fencer

    Fencer Guest

    I rewrote it like this:
    package utility;

    import java.util.Arrays;
    import java.util.HashSet;
    import java.util.LinkedHashMap;
    import java.util.Map;
    import java.util.Set;

    public class MiscUtils {

    public static void main(String[] args) {
    Map<String, String> combos =
    findCombinations(new String[]{"chebi", "kegg.compound", "chebi"});
    System.out.println(combos.size());
    System.out.println(combos);
    }

    public static Map<String, String> findCombinations(String[] strings) {
    Set<String> set = new HashSet<String>(Arrays.asList(strings));

    if (set.size() < strings.length) {
    System.err.println("Input array contained duplicates.");
    strings = (String[])set.toArray();
    }

    Map<String, String> combos = new LinkedHashMap<String, String>();

    for (int i = 0; i < strings.length; i++)
    {
    for (int j = i + 1; j < strings.length; j++)
    {
    combos.put(strings, strings[j]);
    }
    }

    return combos;
    }
    }

    but apparently I can't handle duplicates the way I try handle them,
    because I get following runtime error:
    Input array contained duplicates.
    Exception in thread "main" java.lang.ClassCastException:
    [Ljava.lang.Object; cannot be cast to [Ljava.lang.String;
    at utility.MiscUtils.findCombinations(MiscUtils.java:23)
    at utility.MiscUtils.main(MiscUtils.java:13)

    This line strings = (String[])set.toArray(); is what the runtime system
    is unhappy with.


    - Fencer
     
    Fencer, Jul 29, 2010
    #3
  4. If you need a concept of pair, then you need some class that represents
    it. Why would you want to get rid of it?

    What is the reason you are using a Vector (synchronization)? Why not
    implement your equals() and hashcode() methods of the Pair class, and
    simply use the HashSet for storing pairs? Then you wouldn't have to
    worry about duplicates at all.

    for (int j = i+1;.....
     
    Screamin Lord Byron, Jul 29, 2010
    #4
  5. Fencer

    Fencer Guest

    Ok, thanks Pete!

    - fencer
     
    Fencer, Jul 29, 2010
    #5
  6. Fencer

    Fencer Guest



    Hmm, that loop correct doesn't seem correct (or I did as mistake
    explaining the desired behavior). For the input a, b, c, and d it
    generates the three combinations a:d, b:d, and c:d, but it should be six
    combinations:
    a b c d =>
    a b
    a c
    a d
    b c
    b d
    c d

    - Fencer
     
    Fencer, Jul 29, 2010
    #6
  7. Fencer

    Fencer Guest



    Lol nevermind that, I had introduced another error. Sorry for the noise!
     
    Fencer, Jul 29, 2010
    #7
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.