Conversation
| @@ -1,48 +1,170 @@ | |||
| package core.basesyntax; | |||
|
|
|||
| //import jdk.internal.util.ArraysSupport; | |||
There was a problem hiding this comment.
Please don't add redundant imports and commented lines to your code
|
|
||
|
|
There was a problem hiding this comment.
Please don’t add redundant empty lines to your code
| private static final int def_capacity = 10; | ||
| private static final Object[] empty_arraydata = {}; |
There was a problem hiding this comment.
- Pay attention to constants naming convention
https://mate-academy.github.io/style-guides/java/java.html#s5.2.4-constant-names - Use a full name for variables to make code more clear. For example:
private static final int DEFAULT_CAPACITY = 10;
|
|
||
|
|
There was a problem hiding this comment.
Please don’t add redundant empty lines to your code
| private static final Object[] empty_arraydata = {}; | ||
|
|
||
|
|
||
| private T[] array = (T[]) new Object[def_capacity]; |
There was a problem hiding this comment.
- Use informative names for your variables. For example,
elementDataprovides more insight than anarray - Redundant typecast to generic array. Generic type
Twill be erased toObjectafter compilation, so you can directly use aObject[]
Example below:
private Object[] elementData; - It might be better to have 2 constructors in order to initialize
ArrayListwith default and specified initial capacity, but maybe it's not a required for this task, so I will specify it to be sure. Anyway, you can see how it would look like below:
public ArrayList() {
this.elementData = EMPTY_ARRAYDATA;
}
public ArrayList(int initialCapacity) {
if (initialCapacity < 0) {
throw new IllegalArgumentException("Initial capacity must be great than 0");
}
this.elementData = new Object[initialCapacity];
}
| } | ||
|
|
||
|
|
||
| public static Object[] listToArray(List<?> list) { |
There was a problem hiding this comment.
- This method is not a part of public API, so make it
privateand non-static - Use
Listwith generic typeTinstead of raw type, because we want to be sure we stick to the same type
P.S. you can use@SuppressWarnings("unchecked")to avoid "Unchecked cast: 'java.lang.Object[]' to 'T[]'" warning
Example below:
@SuppressWarnings("unchecked")
public T[] listToArray(List<T> list) {
}
| } | ||
| return tempArray; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
It can cause a NPE, so return an EMPTY_ARRAYDATA instead
| boolean empty = true; | ||
| for (T element : array) { | ||
| if (element != null) { | ||
| empty = false; | ||
| break; | ||
| } | ||
| } | ||
| return empty; |
There was a problem hiding this comment.
You can just check if size == 0. It will be O(1) instead of O(n) time complexity
| found: | ||
| { | ||
| if (element == null) { | ||
| for (; index < size; index++) { | ||
| if (array[index] == null) { | ||
| remove(index); | ||
| break found; | ||
| } | ||
| } | ||
| } else { | ||
| for (; index < size; index++) { | ||
| if (element.equals(array[index])) { | ||
| remove(index); | ||
| break found; | ||
| } | ||
| } | ||
| throw new NoSuchElementException(); | ||
| } | ||
| } | ||
| return element; |
There was a problem hiding this comment.
- It seems like copy of the original
ArrayListusing thefoundlabel. This is generally considered as a bad practice, because it makes code more verbose and harder to read, so I suggest rewrite it simply usingforstatements. - Make your exceptions informative
https://mate-academy.github.io/jv-program-common-mistakes/java-core/collections/array-list.html#make-your-exceptions-informative
| import java.util.Arrays; | ||
| import java.util.NoSuchElementException; | ||
|
|
||
|
|
There was a problem hiding this comment.
Please don’t add redundant empty lines to your code
No description provided.