replace unnecessary owned-type to borrowed-type (String)

[Watagon]

Pull Request

Description

This pull request aims to improve code quality and memory efficiency by promoting the use of borrowed references (&str) over unnecessary ownership (String).

Change function signatures (String -> &str)

• src/dynamic_programming/is_subsequence.rs
• src/dynamic_programming/longest_common_substring.rs
• src/string/knuth_morris_pratt.rs
• src/string/rabin_karp.rs
• src/string/run_length_encoding.rs
• src/string/suffix_tree.rs
• src/string/autocomplete_using_trie.rs
• src/string/burrows_wheeler_transform.rs

Change the code in test (remove unnecessary String construction)

• src/data_structures/rb_tree.rs

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• I ran bellow commands using the latest version of rust nightly.
• I ran cargo clippy --all -- -D warnings just before my last commit and fixed any issue that was found.
• I ran cargo fmt just before my last commit.
• I ran cargo test just before my last commit and all tests passed.
• I checked COUNTRIBUTING.md and my code follows its guidelines.

Question

1. I found some issues when I ran cargo clippy --all -- -D warnings.
   Those issues don't seem related to code I modified. What should I do?

2. Which type is better for code followed

   • (&str, usize), &[&str] (it's not easy to make reference from (String, usize) or Vec<String>)
   • <T: AsRef<str>>(T, usize), <T: AsRef<str>>&[T]
     

     Rust/src/string/autocomplete_using_trie.rs

     Line 79 in e6be8cc

     pub fn insert_words(&mut self, words: Vec<String>) {

     
     

     Rust/src/string/burrows_wheeler_transform.rs

     Line 22 in e6be8cc

     pub fn inv_burrows_wheeler_transform(input: (String, usize)) -> String {

This marks my first-ever contribution, and I would greatly appreciate any guidance or feedback on areas where I may have made mistakes or could improve.

[siriak]

Looks good, thanks!

[siriak]

1. Could you fix them? We are using the latest clippy, and they have added these warnings since the last merged PR.
2. I don't have a preference, you can make whichever way you want. @imp2002 maybe you've got something to add here?

[Watagon]

I've encountered some compilation errors related to the incorrect implementation of partial_cmp on Ord types. Your expertise would be greatly appreciated in resolving these issues.

Here are the details:

error: incorrect implementation of `partial_cmp` on an `Ord` type
  --> src/general/huffman_encoding.rs:29:1
   |
29 | /  impl<T> PartialOrd for HuffmanNode<T> {
30 | |      fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
   | | _____________________________________________________________-
31 | ||         Some(self.frequency.cmp(&other.frequency).reverse())
32 | ||     }
   | ||_____- help: change this to: `{ Some(self.cmp(other)) }`
33 | |  }
   | |__^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#incorrect_partial_ord_impl_on_ord_type
   = note: `#[deny(clippy::incorrect_partial_ord_impl_on_ord_type)]` on by default

error: incorrect implementation of `partial_cmp` on an `Ord` type
  --> src/graph/astar.rs:17:1
   |
17 | /  impl<V: Ord + Copy, E: Ord + Copy> PartialOrd for Candidate<V, E> {
18 | |      fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
   | | _______________________________________________________________________-
19 | ||         // Note the inverted order; we want nodes with lesser weight to have
20 | ||         // higher priority
21 | ||         other.estimated_weight.partial_cmp(&self.estimated_weight)
22 | ||     }
   | ||_____- help: change this to: `{ Some(self.cmp(other)) }`
23 | |  }
   | |__^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#incorrect_partial_ord_impl_on_ord_type

error: could not compile `the_algorithms_rust` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: could not compile `the_algorithms_rust` (lib test) due to 2 previous errors

Should I add a "#[allow(...)]" option since the code was written intentionally, or make new method?

We'd appreciate it if you could take a look at these issues within the context of the existing Pull Request conversation and provide your feedback. If you have any suggestions or concerns, please let us know.

[siriak]

Can't we replace partial_cmp implementation with Some(self.cmp(other))? It should work the same and satisfy the linter

[siriak]

Looks good, thanks!