8000 the sum of two integer may overflow. by advancedxy · Pull Request #37 · soulmachine/leetcode · GitHub
[go: up one dir, main page]
Skip to content

the sum of two integer may overflow. #37

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 2, 2014
Merged

the sum of two integer may overflow. #37

merged 1 commit into from
Dec 2, 2014

Conversation

advancedxy
Copy link
Contributor

@soulmachine
Copy link
Owner

你的这个改动很好，不过本 8000 的代码，更加注重可读性。

如果连简单的 x+y 都要考虑溢出，那代码会变得非常难懂

@soulmachine soulmachine closed this Dec 1, 2014
@advancedxy
Copy link
Contributor Author
  1. 这是个 bug, 一旦数组的大小超过2^30接近2^31-1, 搜索数组后半截的数肯定会失败.
  2. 对于 x + y 取平均数而言, 这个改动可读性并没有降低. 实际上, 任何强类型的语言在做类似的取平均数的时候都应该使用上面的做法.
  3. 本书其他地方取平均数的代码就是使用我改动后的代码, 我当时用 grep 过了一遍.

@soulmachine soulmachine reopened this Dec 2, 2014
@soulmachine
Copy link
Owner

我重新grep了一把，发现以前的确有很多防止溢出的写法，那这个也采纳吧，非常感谢！

soulmachine added a commit that referenced this pull request Dec 2, 2014
@soulmachine
Merge pull request #37 from advancedxy/fixMediumCaculate
4b439f0 
the sum of two integer may overflow.
@soulmachine soulmachine merged commit 4b439f0 into soulmachine:master Dec 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@advancedxy @soulmachine
0