Can you check my task, please? #1214

IvanIKS · 2023-11-06T18:02:17Z

No description provided.

sokimaaa · 2023-11-06T19:12:28Z

src/main/java/core/basesyntax/ArrayList.java

+    private int lastElementIndex;
+
+    public ArrayList() {
+        array = (T[])new Object[10];


10 is magic number, should be extracted into constant.

Suggested change

array = (T[])new Object[10];

array = (T[])new Object[CONSTANT];

sokimaaa · 2023-11-06T19:13:32Z

src/main/java/core/basesyntax/ArrayList.java

+            ++lastElementIndex;
+            array[lastElementIndex] = value;
+        } else {
+            changeSize(1.5);


1.5 is a magic number, make it constant

Suggested change

changeSize(1.5);

changeSize(CONSTANT);

sokimaaa · 2023-11-06T19:14:34Z

src/main/java/core/basesyntax/ArrayList.java

+        if (index < 0) {
+            throw new ArrayListIndexOutOfBoundsException("Invalid index, can't be added to"
+                    + " non-existing position.");
+        }


potentially repeating code, could be extracted into private method

sokimaaa · 2023-11-06T19:15:20Z

src/main/java/core/basesyntax/ArrayList.java

+        if (index > lastElementIndex + 1) {
+            throw new ArrayListIndexOutOfBoundsException("Invalid index, can't be added to"
+                    + " non-existing position.");
+        }


merge with previous out of bound check condition

sokimaaa · 2023-11-06T19:16:42Z

src/main/java/core/basesyntax/ArrayList.java

+        Object[] newArray = new Object[array.length];
+        for (int i = 0; i < index; ++i) {
+            newArray[i] = array[i];
+        }
+        newArray[index] = value;
+        for (int i = index + 1; i <= lastElementIndex + 1; ++i) {
+            newArray[i] = array[i - 1];
+        }
+        ++lastElementIndex;


consider using System.arraycopy to reduce the code

sokimaaa · 2023-11-06T19:17:47Z

src/main/java/core/basesyntax/ArrayList.java

+            for (int i = 0; i < list.size(); ++i) {
+                ++lastElementIndex;
+                array[lastElementIndex] = list.get(i);
+            }


consider reusing of add(value) method.

sokimaaa · 2023-11-06T19:18:14Z

src/main/java/core/basesyntax/ArrayList.java

+        if (index < 0) {
+            throw new ArrayListIndexOutOfBoundsException("You're trying"
+                    + " to get element, index of which is lower than 0");
+        }
+        if (index > lastElementIndex) {
+            throw new ArrayListIndexOutOfBoundsException("You're trying"
+                    + " to get element, index of which is larger than array length.");
+        }


1 merge if statements into one
2 extract private method for reuse purpose

sokimaaa · 2023-11-06T19:19:35Z

src/main/java/core/basesyntax/ArrayList.java

+        if (lastElementIndex == -1) {
+            return true;
+        } else {
+            return false;
+        }


Redundant if statement. lastElementIndex == -1 already returns boolean

Suggested change

if (lastElementIndex == -1) {

return true;

} else {

return false;

}

return lastElementIndex == -1

sokimaaa · 2023-11-06T19:19:56Z

src/main/java/core/basesyntax/ArrayList.java

+        if (newLength > array.length) {
+            for (int i = 0; i < array.length; i++) {
+                tempArray[i] = array[i];
+            }
+        } else if (newLength < array.length) {
+            for (int i = 0; i < newLength; i++) {
+                tempArray[i] = array[i];
+            }
+        }
+        array = (T[]) tempArray;


consider using System.arraycopy

IvanIKS · 2023-11-08T19:19:18Z

I made changes you requested. Can you check it, please?

Rommelua · 2023-11-09T09:10:57Z

src/main/java/core/basesyntax/ArrayList.java

+    private final int baseSize = 10;
+    private final double enlargingSize = 1.5;


Rommelua · 2023-11-09T09:11:36Z

src/main/java/core/basesyntax/ArrayList.java

 public class ArrayList<T> implements List<T> {
+    private T[] array;
+    private int lastElementIndex;


Suggested change

private int lastElementIndex;

private int size;

Rommelua · 2023-11-09T09:11:54Z

src/main/java/core/basesyntax/ArrayList.java

+
+    public ArrayList() {
+        array = (T[])new Object[baseSize];
+        lastElementIndex = -1;


Suggested change

lastElementIndex = -1;

Rommelua · 2023-11-09T09:13:15Z

src/main/java/core/basesyntax/ArrayList.java

+        if (lastElementIndex == array.length - 1) {
+            changeSize(enlargingSize);
+        }
+        Object[] newArray = new Object[array.length];


We do not need newArray in this method

Rommelua · 2023-11-09T09:14:42Z

src/main/java/core/basesyntax/ArrayList.java

    }

    @Override
    public void addAll(List<T> list) {
-
+        int expectedLength = (list.size() + this.size());
+        if (expectedLength <= array.length) {


invert IF condition and get rid of else word

Rommelua · 2023-11-09T09:15:24Z

src/main/java/core/basesyntax/ArrayList.java

+        for (int i = index; i < lastElementIndex; ++i) {
+            array[i] = array[i + 1];
+        }


Rommelua · 2023-11-09T09:16:03Z

src/main/java/core/basesyntax/ArrayList.java

+                index = i;
+                break;
+            } else if (element != null && element.equals(array[i])) {
+                index = i;
+                break;


Don't create repeating code.

Rommelua · 2023-11-09T09:16:27Z

src/main/java/core/basesyntax/ArrayList.java

+        if (newLength > array.length) {
+            for (int i = 0; i < array.length; i++) {
+                tempArray[i] = array[i];
+            }
+        } else if (newLength < array.length) {
+            for (int i = 0; i < newLength; i++) {
+                tempArray[i] = array[i];
+            }
+        }
+        array = (T[]) tempArray;


Rommelua · 2023-11-09T09:17:40Z

src/main/java/core/basesyntax/ArrayList.java

+        return array;
+    }
+
+    private void checkIfArgumentIsCorrect(int argument) {


unclear method name

Suggested change

private void checkIfArgumentIsCorrect(int argument) {

private void checkIndex(int index) {

Elena-Bruyako · 2024-04-24T06:48:18Z

src/main/java/core/basesyntax/ArrayList.java

 public class ArrayList<T> implements List<T> {
+    private T[] array;
+    private int size;
+    private final int baseSize = 10;


Elena-Bruyako · 2024-04-24T06:48:21Z

src/main/java/core/basesyntax/ArrayList.java

+    private T[] array;
+    private int size;
+    private final int baseSize = 10;
+    private final double enlargingSize = 1.5;


Elena-Bruyako · 2024-04-24T06:48:47Z

src/main/java/core/basesyntax/ArrayList.java

+
+    public ArrayList() {
+        array = (T[]) new Object[baseSize];
+        size = 0;


int type = 0 by default

Elena-Bruyako · 2024-04-24T06:49:03Z

src/main/java/core/basesyntax/ArrayList.java

+        size = 0;
+    }
+
+    public ArrayList(T[] array) {


Why do you need this constructor?

Elena-Bruyako · 2024-04-24T06:49:51Z

src/main/java/core/basesyntax/ArrayList.java

 public class ArrayList<T> implements List<T> {
+    private T[] array;


Suggested change

private T[] array;

private T[] elements;

Elena-Bruyako · 2024-04-24T06:54:50Z

src/main/java/core/basesyntax/ArrayList.java

+    }
+
+    private T[] changeSize(double coefficient) {
+        int newLength = (int) (array.length * coefficient);


use CAPACITY_INDEX instead of coefficient

Elena-Bruyako · 2024-04-24T06:55:17Z

src/main/java/core/basesyntax/ArrayList.java

+    private T[] array;
+    private int size;
+    private final int baseSize = 10;
+    private final double enlargingSize = 1.5;


Suggested change

private final double enlargingSize = 1.5;

private static final double CAPACITY_INDEX = 1.5;

src/main/java/core/basesyntax/ArrayList.java

Elena-Bruyako · 2024-04-24T06:57:06Z

src/main/java/core/basesyntax/ArrayList.java

+    }
+
+    private void checkIndex(int index) {
+        if (index < 0) {


Suggested change

if (index < 0) {

if (index < 0 || index >= size) {

throw new ArrayListIndexOutOfBoundsException("There is no such index in the list, "

+ index);

}

Elena-Bruyako · 2024-04-24T06:58:54Z

src/main/java/core/basesyntax/ArrayList.java

+        if (index > size - 1) {
+            throw new ArrayListIndexOutOfBoundsException("You're trying"
+                    + " to access element, index of which is larger than array length.");
+        }


add

private void checkIndex(int index) { if (index < 0 || index > size) { throw new ArrayListIndexOutOfBoundsException("Invalid index, " + index); } }

Rommelua

Previous comments are not fixed

okuzan · 2024-08-03T12:19:02Z

src/main/java/core/basesyntax/ArrayList.java

+        int index = -1;
+        for (int i = 0; i < size; ++i) {
+            if (element == elements[i]
+                    | (element != null && element.equals(elements[i]))) {


Suggested change

| (element != null && element.equals(elements[i]))) {

|| (element != null && element.equals(elements[i]))) {

always use short-circuiting operators

okuzan · 2024-08-03T12:20:28Z

src/main/java/core/basesyntax/ArrayList.java

+            throw new ArrayListIndexOutOfBoundsException("There is no such element in the list.");
+        }
+    }
+
+    private void checkAddIndex(int index) {
+        if (index < 0 || index > size + 1) {
+            throw new ArrayListIndexOutOfBoundsException("Invalid index, can't add element to"
+                    + " non-existing position.");
+        }


include index and size in the message

okuzan · 2024-08-03T12:20:40Z

src/main/java/core/basesyntax/ArrayList.java

+            throw new NoSuchElementException("You're trying to remove element,"
+                    + " that is not present in the list");


include element in the message

# Conflicts: # src/main/java/core/basesyntax/ArrayList.java

Elena-Bruyako · 2024-08-05T13:55:17Z

src/main/java/core/basesyntax/ArrayList.java

+        checkAddIndex(index);
+        if (index == 0 && size == 0) {
+            add(value);
+            return;
+        }
+        if (size >= elements.length) {
+            resize();
+        }
+        size++;
+        T[] temp = (T[]) (new Object[elements.length]);
+        System.arraycopy(elements, 0, temp, 0, index);
+        temp[index] = value;
+        System.arraycopy(elements, index, temp, index + 1, elements.length - index - 1);
+        elements = (T[]) temp; //without this array does not resize properly.


Suggested change

checkAddIndex(index);

if (index == 0 && size == 0) {

add(value);

return;

}

if (size >= elements.length) {

resize();

}

size++;

T[] temp = (T[]) (new Object[elements.length]);

System.arraycopy(elements, 0, temp, 0, index);

temp[index] = value;

System.arraycopy(elements, index, temp, index + 1, elements.length - index - 1);

elements = (T[]) temp; //without this array does not resize properly.

checkAddIndex(index);

resize();

System.arraycopy(elements, index, elements, index + 1, size - index);

elements[index] = value;

size++;

Elena-Bruyako · 2024-08-05T13:55:34Z

src/main/java/core/basesyntax/ArrayList.java

+        if (size >= elements.length) {
+            resize();
+        }


Suggested change

if (size >= elements.length) {

resize();

}

resize();

Elena-Bruyako · 2024-08-05T13:56:18Z

src/main/java/core/basesyntax/ArrayList.java

    }

    @Override
    public T remove(T element) {
-        return null;
+        int index = -1;


Suggested change

int index = -1;

int index = index(element);

if (index == -1) {

throw new NoSuchElementException("There is no such element in the list, " + element);

}

return remove(index);

Elena-Bruyako · 2024-08-05T13:56:34Z

src/main/java/core/basesyntax/ArrayList.java

+        return size == 0;
+    }
+
+    private void resize() {


Suggested change

private void resize() {

private void resize() {

if (elements.length == size) {...

# Conflicts: # src/main/java/core/basesyntax/ArrayList.java

Elena-Bruyako · 2024-08-07T13:15:19Z

src/main/java/core/basesyntax/ArrayList.java

+
+    private int findElement(T element) {
+        for (int i = 0; i < size; ++i) {
+            if (Objects.equals(element, elements[i])) {


Don't use Objects, Arrays, or any other util class.

Elena-Bruyako · 2024-08-07T13:15:37Z

src/main/java/core/basesyntax/ArrayList.java

+    }
+
+    private void checkAddIndex(int index) {
+        if (index < 0 || index > size + 1) {


Suggested change

if (index < 0 || index > size + 1) {

if (index < 0 || index > size) {

IvanIKS · 2024-08-07T14:59:20Z

Sorry, Intellij Idea told me to replace my longer "element == <...>" with Object.equals. I changed back. Can you check again now, please?

okuzan

On real job, you will use Objects.equals, it's forbidden to use in these homework (up to hashmap) only to teach students how to write the code themselves, good job

boroda4436 and others added 4 commits November 6, 2023 20:01

update checklist.md (mate-academy#1182)

1a87a5d

Merge remote-tracking branch 'origin/master'

939b3b6

Merge remote-tracking branch 'origin/master'

58c52b9

Merge remote-tracking branch 'origin/master'

98d410a

sokimaaa suggested changes Nov 6, 2023

View reviewed changes

IvanIKS added 4 commits November 8, 2023 21:09

Merge remote-tracking branch 'origin/master'

f2d3fa2

Merge remote-tracking branch 'origin/master'

4ec2d52

Merge remote-tracking branch 'origin/master'

030b08e

Merge remote-tracking branch 'origin/master'

c7f9151

IvanIKS requested a review from sokimaaa November 8, 2023 19:18

Rommelua suggested changes Nov 9, 2023

View reviewed changes

IvanIKS added 4 commits April 24, 2024 01:06

Merge remote-tracking branch 'origin/master'

bea0209

Merge remote-tracking branch 'origin/master'

2216abf

Merge remote-tracking branch 'origin/master'

b5ad7f3

Merge remote-tracking branch 'origin/master'

d89778c

IvanIKS requested a review from Rommelua April 23, 2024 22:10

Elena-Bruyako suggested changes Apr 24, 2024

View reviewed changes

IvanIKS added 2 commits April 24, 2024 19:15

Merge remote-tracking branch 'origin/master'

a229576

Merge remote-tracking branch 'origin/master'

6c9a798

IvanIKS requested a review from Elena-Bruyako April 25, 2024 08:14

Rommelua suggested changes Apr 25, 2024

View reviewed changes

IvanIKS added 3 commits April 28, 2024 09:19

Fixed problems.

c5bd73d

Merge remote-tracking branch 'origin/master'

772eb65

Merge remote-tracking branch 'origin/master'

833a81e

IvanIKS requested a review from Rommelua August 3, 2024 10:39

okuzan suggested changes Aug 3, 2024

View reviewed changes

Merge remote-tracking branch 'origin/master'

bfcc2ba

IvanIKS requested a review from okuzan August 4, 2024 20:20

Merge remote-tracking branch 'origin/master'

459bafa

Merge remote-tracking branch 'origin/master'

0245dfd

# Conflicts: # src/main/java/core/basesyntax/ArrayList.java

Elena-Bruyako suggested changes Aug 5, 2024

View reviewed changes

IvanIKS added 6 commits August 6, 2024 22:35

Merge remote-tracking branch 'origin/master'

45c2358

# Conflicts: # src/main/java/core/basesyntax/ArrayList.java

Merge remote-tracking branch 'origin/master'

e20ed6f

Merge remote-tracking branch 'origin/master'

83b79f5

Merge remote-tracking branch 'origin/master'

611d5ff

Merge remote-tracking branch 'origin/master'

388e486

Merge remote-tracking branch 'origin/master'

bfd7fcb

IvanIKS requested a review from Elena-Bruyako August 6, 2024 19:39

Elena-Bruyako suggested changes Aug 7, 2024

View reviewed changes

IvanIKS added 2 commits August 7, 2024 17:54

Merge remote-tracking branch 'origin/master'

5cf1b11

Merge remote-tracking branch 'origin/master'

68bc970

IvanIKS requested a review from Elena-Bruyako August 7, 2024 14:59

okuzan approved these changes Aug 8, 2024

View reviewed changes

	array = (T[])new Object[10];
	array = (T[])new Object[CONSTANT];

		private final int baseSize = 10;
		private final double enlargingSize = 1.5;

	private void checkIfArgumentIsCorrect(int argument) {
	private void checkIndex(int index) {

		public class ArrayList<T> implements List<T> {
		private T[] array;

	private final double enlargingSize = 1.5;
	private static final double CAPACITY_INDEX = 1.5;

-        if (index < 0) {
+       if (index < 0 || index >= size) {
+            throw new ArrayListIndexOutOfBoundsException("There is no such index in the list, "
+                    + index);
+        }

	\| (element != null && element.equals(elements[i]))) {
	\|\| (element != null && element.equals(elements[i]))) {

		throw new NoSuchElementException("You're trying to remove element,"
		+ " that is not present in the list");

-        int index = -1;
+        int index = index(element);
+        if (index == -1) {
+            throw new NoSuchElementException("There is no such element in the list, " + element);
+        }
+        return remove(index);

	private void resize() {
	private void resize() {
	if (elements.length == size) {...

	if (index < 0 \|\| index > size + 1) {
	if (index < 0 \|\| index > size) {

Can you check my task, please? #1214

Are you sure you want to change the base?

Can you check my task, please? #1214

Conversation

IvanIKS commented Nov 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvanIKS commented Nov 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rommelua left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvanIKS commented Aug 7, 2024

okuzan left a comment

Choose a reason for hiding this comment