Skip to content

[Hamill] 코드 리뷰 요청 #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
3 changes: 2 additions & 1 deletion .idea/gradle.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

124 changes: 124 additions & 0 deletions .idea/uiDesigner.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified hamill/.gradle/5.2.1/executionHistory/executionHistory.bin
Binary file not shown.
Binary file modified hamill/.gradle/5.2.1/executionHistory/executionHistory.lock
Binary file not shown.
Binary file modified hamill/.gradle/5.2.1/fileHashes/fileHashes.bin
Binary file not shown.
Binary file modified hamill/.gradle/5.2.1/fileHashes/fileHashes.lock
Binary file not shown.
Binary file modified hamill/.gradle/buildOutputCleanup/buildOutputCleanup.lock
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.codesquad.datastructurestudy.basic;

public class BinarySearch {

private static int binarySearch(int[] numbers, int number, int first, int last) {
while (first <= last) {
int mid = (first + last) / 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 int overflow를 조심하셔야해요.
https://endorphin0710.tistory.com/112
여길 참고해보세요.


if (numbers[mid] == number) {
return mid;
} else if (numbers[mid] < number) {
first = mid + 1;
} else {
last = mid - 1;
}
}
return -1;
}

public static void main(String[] args) {
int[] numbers = {1,2,3,4,5,6,7,8,9};
int number = 2;
int index = binarySearch(numbers, number, 0, numbers.length - 1);

if (index == -1) {
System.out.println("못찾았다");
} else {
System.out.println(index + "번째 인덱스에서 찾았다");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.codesquad.datastructurestudy.basic;

import java.util.Arrays;

public class BinarySearchOperationCount {

private static int binarySearch(int[] numbers, int number, int first, int last) {
int mid;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 약간 윤성우님 책 스타일로 선언을 했네요 ㅋㅋ
저라면 초기화와 동시에 선언하는 스타일을 사용했을 것 같습니다.

int opCount = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 윤성우 책에서 안좋아하는 부분이 명확하지 않은 변수명이에요.
약어를 사용하시기 보다는 보다 명확한 변수명을 사용하시는게 좋을 것 같아요.


while (first <= last) {
mid = (first + last) / 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 리뷰 참고해서 수정하시길 바랄게요!


if (numbers[mid] == number) {
return mid;
} else if (numbers[mid] < number) {
first = mid + 1;
} else {
last = mid - 1;
}
opCount++;
}
return opCount;
}

public static void main(String[] args) {
int[] arr1 = new int[500];
int[] arr2 = new int[5000];
int[] arr3 = new int[50000];
int number = 5000;

Arrays.fill(arr1, 0);
Arrays.fill(arr2, 0);
Arrays.fill(arr3, 0);

int opCount1 = binarySearch(arr1, number, 0, arr1.length - 1);
int opCount2 = binarySearch(arr2, number, 0, arr2.length - 1);
int opCount3 = binarySearch(arr3, number, 0, arr3.length - 1);

System.out.println("비교연산횟수 : " + opCount1);
System.out.println("비교연산횟수 : " + opCount2);
System.out.println("비교연산횟수 : " + opCount3);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.codesquad.datastructurestudy.basic;

public class LinearSearch {

private static int linearSearch(int[] numbers, int number) {
int i;
for (i = 0; i < numbers.length; i++) {
Comment on lines +6 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗... 아아...

if (numbers[i] == number) {
return i;
}
}
return -1;
}

public static void main(String[] args) {
int[] arr = {1,2,3,4,5};
int number = 2;
int index = linearSearch(arr, number);

if (index == -1) {
System.out.println("못찾았다");
} else {
System.out.println((index+1) + "번째 에서 찾았다");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
System.out.println((index+1) + "번째 에서 찾았다");
System.out.println((index + 1) + "번째 에서 찾았다");

사소한거지만, 이런건 띄어쓰는게 관례입니다.

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package com.codesquad.datastructurestudy.list.arraylist;

public class ArrayList implements List<Integer> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

특이하네요 ㅋㅋㅋ 제네릭을 이렇게 구현하신 이유가 있을까요?


private int[] numbers;
private int numOfData;

public ArrayList() {
this.numbers = new int[100];
this.numOfData = 0;
Comment on lines +9 to +10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 가급적이면 숫자 상수도 private static final로 빼서 사용하는 편이 좋지 않을까 싶어요.
그리고 numOfData는 그냥 제거해도 0으로 초기화 되니 그 점을 염두에 두시는게 좋을 것 같네요.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가로...
저라면 생성자를

public ArrayList(int initialSize) {
  this.numbers = new int[initialSize];
}

이런 식으로 만들고 생성자 오버로딩을 통해서

public ArrayList() {
  this(INITIAL_SIZE);
}

와 같은 식으로 구현해봤을 것 같네요.

}

public ArrayList(int[] numbers, int numOfData) {
this.numbers = numbers;
this.numOfData = numOfData;
}
Comment on lines +13 to +16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

살짝 애매한 생성자 같아요.
이 생성자가 과연 필요할까? 라는 생각도 들고요.


public void add(int index, Integer element) {
if (numOfData > this.numbers.length) {
System.out.println("배열이 가득 찼습니다");
// throw new ArrayIndexOutOfBoundsException();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안쓰는 주석은 제거해주세요~

}
this.numbers[index] = element;
numOfData++;
}
Comment on lines +18 to +25
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 메서드 확실한가요?
행동하는 것은 replace에 가까운데, 하는 동작에 군더더기가 많이 들어간 느낌입니다.
특히 개수 체크하는 로직은 없어도 될 것 같고, 마찬가지로 numOfData++; 도 불필요해보입니다.
그리고 IndexOutOfBoundException도 발생가능한 코드입니다.

  1. add인데, 만약 0번 위치에 계속 데이터를 집어넣으면? 나중엔 배열이 가득 찼다고 하면서 데이터는 바뀌고, numOfData도 계속 증가할겁니다. if문이 종료되고 아래 라인들이 실행될 필요가 없다면 return문을 사용해주세요.
  2. numOfData라는 멤버변수에 this 키워드를 추가해주세요. 헷갈릴 수 있습니다.
  3. 23번 라인에 IndexOutOfBoundException이 발생가능한 코드가 그대로 있습니다. 예외 처리를 추가해주시던지, if 문으로 조건을 달아주세요. (인덱스의 범위를 생각해봅시다.)
  4. 23번 라인에 element가 unboxing될 때, 어떻게 될지도 생각해봅시다. null이 그대로 들어가게 된다면? 어떤 결과가 벌어질까요?


public void add(Integer element) {
if (numOfData > this.numbers.length) {
System.out.println("배열이 가득 찼습니다");
}
this.numbers[numOfData] = element;
numOfData++;
}
Comment on lines +27 to +33
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 마찬가지입니다. 위의 코멘트를 참고해서 정확한 로직으로 변경해주세요.


public int count() {
return this.numOfData;
}
Comment on lines +35 to +37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메서드명은 관례상 size()가 좀 더 적합해보입니다.


public Integer get(int index) {
return this.numbers[index];
}
Comment on lines +39 to +41
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrapper type으로 반환하게 되면 auto boxing이 일어나기 때문에
여기선 불필요하다고 보입니다.
Integerint 타입은 많은 차이가 있습니다.
특히 메모리 공간, Nullable 등의 차이가 있겠네요.


public Integer remove(int index) {
Integer data = this.numbers[index];

for (int i = index; i < this.numOfData - 1; i++) {
this.numbers[i] = this.numbers[i+1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.numbers[i] = this.numbers[i+1];
this.numbers[i] = this.numbers[i + 1];

}

this.numOfData--;
return data;
}
Comment on lines +43 to +52
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

데이터가 없다면 0을 주겠네요?
뭔가 이상하지 않을까요 ㅎㅎ


public int size() {
return this.numbers.length;
}
Comment on lines +54 to +56
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

응? 이거 어떻게보면 맞는 것 같기도 하고 아닌 것 같기도 합니다.
java의 size는 어떻게 동작하나요?


public static void main(String[] args) {
List<Integer> list = new ArrayList();

// 데이터 삽입
list.add(1);
list.add(2);
list.add(3);
list.add(4);

// 저장된 데이터 출력
System.out.println("현재 데이터의 수: " + list.count());

for (int i = 0; i < list.count(); i++) {
System.out.println(list.get(i));
}

list.remove(2);
list.remove(3);

System.out.println("삭제 후 데이터의 수: " + list.count());

for (int i = 0; i < list.count(); i++) {
System.out.println(list.get(i));
}

System.out.println("삭제 후 배열 크기: " + list.size());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.codesquad.datastructurestudy.list.arraylist;

public interface List<T> {

void add(T element);
void add(int index, T element);

int count();

T get(int index);

T remove(int index);

int size();

// int indexOf(T o);
// int lastIndexOf(T o);

// ListIterator listIterator();
// ListIterator listIterator(int index);

// T set(int index, T element);

// void sort(Comparator c);

// List subList(int fromIndex, int toIndex);
Comment on lines +16 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅋㅋㅋ 나중에 구현할 사항들인가요?

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.codesquad.datastructurestudy.list.arraylist;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;

public class ListMain {

public static void main(String[] args) {
List<Integer> list = new ArrayList<>();
for (int i = 0; i < 9; i++) {
list.add(i,i+1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전반적으로 java coding convention이 잘 지켜지지 않는 모습이네요.

}

int sum = 0;
for (int i = 0; i < list.size(); i++) {
sum += list.get(i);
}
System.out.println("리스트의 합계는 : " + sum);

List list1 = new ArrayList();

for (int i = 0; i < list.size(); i++) {
if (!(list.get(i) % 3 == 0 || list.get(i) % 2 == 0)) {
list1.add(list.get(i));
}
}

Iterator<Integer> iterator = list1.iterator();
while (iterator.hasNext()) {
System.out.println(iterator.next());
}
Comment on lines +30 to +33
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enhanced for loop를 사용해도 됩니다.

}
}
Loading