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

Avoid Clone in FenwickTree #104

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

Conversation

TonalidadeHidrica
Copy link
Collaborator

Current implementation of FenwickTree clones the values to be added, but in principle, these operations can be done by just passing reference. I changed the trait bound so that it now requires reference values can be added, and removed the Clone bound.

This PR depends on #102 (this dependency is not actually necessary, but when it comes to combining these two, it will require a certain amount of rollbacks, so I created a new branch from that branch.)

@@ -1,35 +1,37 @@
use std::iter::repeat_with;

use crate::num_traits::Zero;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These appear to be changes that need not specifically rely on #102. Check out the application examples below.

https://github.com/rust-lang-ja/ac-library-rs/compare/7e99fd2941976764b4e7925898fae7cc77890885...mizar:ac-library-rs:4be604494341a86242b77120e5685740dc41791f?expand=1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think requiring using Default is not always a good idea because the semantics are different. Zero traits is clearly for the "additive identity", while Default trait is for a general "default" instance. The latter is not guaranteed to be an additive identity, and that restriction shall be stated in the doc instead of as a type restriction. I prefer type restrictions to other document constraints, and thus thought introducing Zero trait would be reasonable.

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