Skip to content

Use List<Boolean> instead of boolean[] in Set#1

Open
sim642 wants to merge 1 commit intout-aa:masterfrom
sim642:boolean-list
Open

Use List<Boolean> instead of boolean[] in Set#1
sim642 wants to merge 1 commit intout-aa:masterfrom
sim642:boolean-list

Conversation

@sim642
Copy link
Copy Markdown

@sim642 sim642 commented Sep 20, 2016

BitVectorGenerator interface uses the type Set<boolean[]>. The problem with this type is demonstrated by the following example:

Set<boolean[]> wat = new HashSet<>();
boolean[] arr = {true};
wat.add(arr);
wat.add(arr);
wat.add(new boolean[]{true});
System.out.println(wat.size()); // 2

(running example)

All arrays (including those of primitives) are objects which do not override equals method. This means Object.equals is used which doesn't compare the content of arrays. This fact makes the use of boolean[] in a Set essentially useless because it's neither correct nor reliable as all of Set's methods rely on correct implementation of equals.

Instead List<Boolean> has properly implemented equals method, fixing the issue above:

Set<List<Boolean>> noWat = new HashSet<>();
List<Boolean> list = Arrays.asList(true);
noWat.add(list);
noWat.add(list);
noWat.add(Arrays.asList(true));
System.out.println(noWat.size()); // 1

(running example)

In addition Lists are much easier to manipulate than arrays, making the task less about reinventing the wheel which is List.

@kspar
Copy link
Copy Markdown
Member

kspar commented Sep 21, 2016

Thanks for the suggestion! I agree that using a List instead of an array as a generic type of a Set (and other Collections) is pretty much always more reasonable. But since changing this at the moment is costly because many people have already implemented it can you give an example as to why changing this is essential in the context of this exercise? If it's not essential then it will be changed before next year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants