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

Add support for the FenwickTree#sum with open ended ranges #23

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

ngng628
Copy link
Contributor

@ngng628 ngng628 commented Sep 3, 2023

  • Tests pass
  • Appropriate changes to README are included in PR

Overview

  • 標準ライブラリに合わせて Range で open ended ranges に対応させました。
    • tree[..3]
    • tree[1..]
    • tree[...]

@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2023

Codecov Report

Merging #23 (9dfaca0) into master (2832b71) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 9dfaca0 differs from pull request most recent head f81b63f. Consider uploading reports for the commit f81b63f to get more accurate results

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
+ Coverage   81.95%   81.99%   +0.03%     
==========================================
  Files          14       14              
  Lines        1092     1094       +2     
==========================================
+ Hits          895      897       +2     
  Misses        197      197              
Files Changed Coverage Δ
src/fenwick_tree.cr 88.88% <100.00%> (+0.88%) ⬆️

Comment on lines 71 to 72
tree[0...].should eq 55
tree[...11].should eq 55
Copy link
Owner

Choose a reason for hiding this comment

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

[SHOULD] 境界値が分かりづらいので、全部ではなく途中からの値を取ってくるテストを追加してください ([...5]など)

[SHOULD] inclusiveかつstart側がopen-endedなテストを追加してください

Copy link
Contributor Author

Choose a reason for hiding this comment

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

追加しました!確認お願いします。

Copy link
Owner

@hakatashi hakatashi left a comment

Choose a reason for hiding this comment

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

ありがとうございます!

@hakatashi hakatashi merged commit 8cf6d4f into hakatashi:master Sep 7, 2023
5 checks passed
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.

3 participants