2

This function should return true only if the parameter object is a subset of the calling object but it always returns true. How to fix it?

    public boolean contains(FileCollection other) {
        int i = 0;
        int j = 0;
        for (i = 0; i<other.files.length; i++) {
            for (j = 0; j<this.files.length; j++) {
                if ((other.files[i]).equals((this.files[j]))) //this refers to the equals method defined in File class
                    break;
            }
            if (j==this.files.length) 
                return false;
        }
        return true;//this method is in FileCollection class
    }
5
  • @vincrichaud it is very clear- I said it always returns true. But the answer needs to be true only if the parameter object is a subset of the calling object Commented Apr 2, 2019 at 10:19
  • Now it is, thanks for the edit Commented Apr 2, 2019 at 10:22
  • Your inner loop exits when j = this.files.length + 1 Commented Apr 2, 2019 at 10:28
  • @vincrichaud then what should it be? Commented Apr 2, 2019 at 10:35
  • The loop exits when the condition is no more matches : when j = this.files.length +1 .In your code, you consider it exits when j = this.files.length. To make your code okay, you just have to change the if condition to the same as the for condition Commented Apr 2, 2019 at 12:05

5 Answers 5

3

(Since you didn't explicitly express what the data type of the array elements is, I'll assume it's File, inferred from comments.)

If you don't mind converting between data structures, maybe converting your arrays (temporarily) to Collections is the most simple way. For example, converting to List:

/* @param other
 * @return true if the calling object contains
 * all files in the parameter object, false otherwise
 */
public boolean contains(FileCollection other) {
    List<File> myList = Arrays.asList(this.files);
    List<File> otherList = Arrays.asList(other.files);
    return myList.containsAll(otherList);
}

Based on your clarify of what to be considered as "contains" when duplicated items are allowed, I'd say you need to count the number of existence for each element. Here is how:

Based on the answer of @Eritrean , you can get and store the count to a map. I made modifications to check the count too:

public boolean contains(FileCollection other) {
    Map<File,Integer> otherFrequency = Arrays.stream(other.files)
            .collect(Collectors.toMap(Function.identity(), v->1,Integer::sum));

    Map<File,Integer> thisFrequency  = Arrays.stream(this.files) 
            .collect(Collectors.toMap(Function.identity(), v->1,Integer::sum));

    if (thisFrequency.entrySet().containsAll(otherFrequency).entrySet()) {
        for (File entry : otherFrequency.entrySet()) {
            if (thisFrequency.get(entry) < otherFrequency.get(entry))
                return false;
        }
        return true;
    }
    return false;
}
Sign up to request clarification or add additional context in comments.

15 Comments

Was writing exactly the same suggestion :-)
Change List into List<File>
it is always returning false
@BhushanOza So are there any duplicated elements in each array?
@renyuneyun yes it can have duplicated items
|
2

For other.files contains this.files to hold, every this.file must be in other.files.

    for (int j = 0; j < this.files.length; j++) {
        boolean found = false;
        for (int i = 0; i < other.files.length; i++) {
            if (other.files[i].equals(this.files[j])) {
                found = true;
                break;
            }
        }
        if (!found) { 
            return false;
        }
    }
    return true;

Not knowing the class of files, probably you can do:

    for (String file : this.files) {
        boolean found = false;
        for (String otherFile : other.files) {
            if (otherFile.equals(file)) {
                found = true;
                break;
            }
        }
        if (!found) { 
            return false;
        }
    }
    return true;

Or even

    for (String file : this.files) {
        boolean found = other.files.indexOf(file) != -1;
        if (!found) { 
            return false;
        }
    }
    return true;

There are nicer datastructures that speed things up, and have predefined methods for things like contains.


With duplicates

    Comparator<File> comparator = new Comparator<File>() {
        @Override
        public int compare(File lhs, File rhs) {
            int cmp = lhs.getBase().compareIgnoreCase(rhs.getBase());
            if (cmp == 0) {
               cmp = lhs.getExtension().compareIgnoreCase(rhs.getExtension());
            }
            if (cmp == 0) {
               cmp = Long.compare(lhs.getSize(), rhs.getSize());
            }
            return cmp;
        }
    };

    Arrays.sort(this.files, comparator);
    Arrays.sort(other.files, comparator);
    int otherI = 0;
    for (File file : this.files.length) {
        boolean found = false;
        while (otherI < other.files.length) {
            int comparison = comparator.compare(other.files[otherI], file);
            ++otherI;
            if (comparison >= 0) {
                found = comparison == 0;
                break;
            }
        }
        if (!found) { 
            return false;
        }
    }
    return true;

By sorting both arrays you can synchronize the comparison at locations in both arrays. The above handles duplicates.

15 Comments

Using your first answer, one of tests fail- when both arrays have a duplicate each. It should be true but it comes out as false
In that case sorting makes sense, then counting & checking duplicates is easier. Arrays.sort.
Arrays.sort() does not work. The File class has many attributes like name, size, etc. It gives ClassCastException
the Arrays.sort() raises ClassCastException
I saw your comment too late; added a Comparator. Not sure about the class File, neither what field to compare.
|
1

Apart from the @renyuneyun suggestion to convert your arrays into Lists you could also make use of the String contains method

public boolean contains(FileCollection other) {
String myList = Arrays.toString(this.files);
String otherList = Arrays.toString(other.files);
return myList.contains(otherList);
}

Of course both of these suggestions are not the optimum solutions from the complexity point of view, but are for sure the shortests :)

Comments

1

What about using a map with File as key and frequency as value:

public boolean contains(FileCollection other) {
    Map<File,Integer> otherFrequency = Arrays.stream(other.files)
            .collect(Collectors.toMap(Function.identity(), v->1,Integer::sum));

    Map<File,Integer> thisFrequency  = Arrays.stream(this.files) 
            .collect(Collectors.toMap(Function.identity(), v->1,Integer::sum));

    return thisFrequency.entrySet().containsAll(otherFrequency.entrySet());
}

Comments

0

Only this answer works for me: (Credit to @Joop Eggen for the Comparator part)

public boolean contains(FileCollection other) {
    Comparator<File> comparator = new Comparator<File>() {
        @Override
        public int compare(File lhs, File rhs) {
            int cmp = lhs.getBase().compareToIgnoreCase(rhs.getBase());
            if (cmp == 0) {
               cmp = lhs.getExtension().compareToIgnoreCase(rhs.getExtension());
            }
            if (cmp == 0) {
               cmp = Long.compare(lhs.getSize(), rhs.getSize());
            }
            if (cmp == 0) {
               cmp = Long.compare(lhs.getPermissions(), rhs.getPermissions());
            }
            return cmp;
        }
    };
    Arrays.sort(this.files, comparator);
    Arrays.sort(other.files, comparator); //THIS AND THE COMPARATOR SORT THE ARRAYS BASED ON ALL FILE ATTRIBUTES    
    int i = 0;
    int j = 0;
    if (this.files.length<other.files.length)
        return false;
    while (i<other.files.length && j<this.files.length) {
        if (!(this.files[j].equals(other.files[i])))
            j++;
        else {
            j++;
            i++;
        }
    }
    if (i<other.files.length)
        return false;
    else
        return true; 
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.