Skip to content
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

Feature/string/add binary #1

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

before30
Copy link

Description

string/add binary 문제 풀이 추가했어요.

Related PRs or Issues

Related algorithms

Check List

  • Remove corresponding python source code
  • Write scalatest code
  • Update README.md list and its link

Copy link
Owner

@kimxogus kimxogus left a comment

Choose a reason for hiding this comment

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

훌륭한 구현 감사합니다 👍

다만 몇가지 요청/의논 했으면 하는 점이 있는데, 코멘트 보시고 의견 부탁드려요 :)

- [x] [MergeIntervals](src/main/scala/algorithms/array/MergeIntervals.scala)
- [x] [garage](src/main/scala/algorithms/array/Garage.scala)
- [longest_non_repeat](python/array/longest_non_repeat.py/)
- [merge_intervals](python/array/merge_intervals.py)
Copy link
Owner

Choose a reason for hiding this comment

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

master 브랜치에 업데이트된 부분이 되돌아가있네요. 수정부탁드려요


*/
println(calc(mutable.MutableList(1, 2, 3, 0, 4), List(0, 3, 2, 1, 4)))
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

기존의 구현이 잘못된 것이 아니라면 굳이 빼지 않고 새로운 구현을 추가해나가는 형태로 하는것이 어떨까 합니다.

Garage object 내에 함수를 다르게 두거나 같은 파일 내에 구현에 따라 object 파일을 따로 두는 방식으로 변경해주실 수 있나요?

main 함수의 코드는 테스트 목적으로 두신 것이라면 test 코드로 옮겨주시는것은 어떨까요?

*/
object LongestSubstring {

}
Copy link
Owner

Choose a reason for hiding this comment

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

이부분은 구현이 빠져있네요

kimxogus pushed a commit that referenced this pull request May 23, 2017
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