Run Rustfmt, a tool for automatically formatting Rust code to the community-accepted style.
Run Clippy, a tool for finding common mistakes that may not be compilation errors but are unlikely to be what the programmer intended.
Don't accept a
&String
— Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument?Don't use explicit
return
keywords at the end of blocks.Instead of
if let Some(_) = max { max.unwrap()
bind the variable:
if let Some(x) = max { x
Your whole last conditional can be simplified to
max.unwrap_or_default()
.Add some automated tests.
#[test] fn example_1() { assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD"); } #[test] fn example_2() { assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB"); } #[test] fn example_3() { assert_eq!(longest_con_seq("aaaa", "aa"), "aa"); } #[test] fn example_4() { assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA"); }
As before, don't match on
Some
and thenunwrap
, bind the variable. We can also avoid the clone by matching on a reference:match max { Some(_) => { let max_str = max.clone(); let max_str = max_str.unwrap();
match &max { Some(max_str) => {
current.clear()
is used in both match arms; extract it.max = Some(current.clone())
is the same; extract it and use a boolean.let do_it = match &max { Some(max_str) => { max_str.len() < current.len() } None => { true } }; if do_it { max = Some(current.clone()); }
This construct can be replaced by
as_ref
andmap_or
:if max.as_ref().map_or(true, |s| s.len() < current.len()) { max = Some(current.clone()); }
#[macro_use]
extern crate criterion;
use criterion::Criterion;
fn longest_con_seq(s1: &str, s2: &str) -> String {
let mut max: Option<String> = None; // Holds value of string with maximum length
let mut current = String::new(); // String container to hold current longest value
let mut s1_iter = s1.chars().peekable(); // Peekable iterator for string s1
let mut s2_iter = s2.chars(); //Iterator for string s2
let mut s2_prev_pos = s2_iter.clone(); // Iterator that holds position of previous location of first iterator
let mut s1_prev_pos = s1_iter.clone(); // Peekable iterator used to make sure all possible combinations are located.
loop {
let s1_char = s1_iter.next(); // Get character in s1
if current.is_empty() {
// If no consecutive string found yet store location of iterator
s1_prev_pos = s1_iter.clone()
}
match s1_char {
Some(s1_char) => loop {
match s2_iter.next() {
Some(s2_char) if s1_char == s2_char => {
current.push(s1_char);
s2_prev_pos = s2_iter.clone();
break;
}
Some(_) => continue,
None => {
s2_iter = s2_prev_pos.clone();
break;
}
}
},
None => match s1_prev_pos.peek() {
Some(_) => {
match &max {
if Somemax.as_ref(max_str) => {
.map_or(true, if|s| max_strs.len() < current.len()) {
max = Some(current.clone());
}
current.clear();
}
None => {
max = Some(current.clone());
current.clear();
}
}
s1_iter = s1_prev_pos.clone();
s2_iter = s2.chars();
}
None => break,
},
}
}
max.unwrap_or_default()
}
fn criterion_benchmark(c: &mut Criterion) {
let s1 = "GXTKAYB";
let s2 = "AGGTAB";
c.bench_function("Benchmark", move |b| b.iter(|| longest_con_seq(s1, s2)));
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
#[test]
fn example_1() {
assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD");
}
#[test]
fn example_2() {
assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB");
}
#[test]
fn example_3() {
assert_eq!(longest_con_seq("aaaa", "aa"), "aa");
}
#[test]
fn example_4() {
assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA");
}
With numerous copies of iterators made and copies of Strings being made
Copies of iterators are generally going to be extremely lightweight. In many cases, they are equivalent to a pointer and and offset.
Cloning a String
is slightly less ideal, but that may just be a restriction of the algorithm.
Run Rustfmt, a tool for automatically formatting Rust code to the community-accepted style.
Run Clippy, a tool for finding common mistakes that may not be compilation errors but are unlikely to be what the programmer intended.
Don't accept a
&String
— Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument?Don't use explicit
return
keywords at the end of blocks.Instead of
if let Some(_) = max { max.unwrap()
bind the variable:
if let Some(x) = max { x
Your whole last conditional can be simplified to
max.unwrap_or_default()
.Add some automated tests.
#[test] fn example_1() { assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD"); } #[test] fn example_2() { assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB"); } #[test] fn example_3() { assert_eq!(longest_con_seq("aaaa", "aa"), "aa"); } #[test] fn example_4() { assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA"); }
As before, don't match on
Some
and thenunwrap
, bind the variable. We can also avoid the clone by matching on a reference:match max { Some(_) => { let max_str = max.clone(); let max_str = max_str.unwrap();
match &max { Some(max_str) => {
#[macro_use]
extern crate criterion;
use criterion::Criterion;
fn longest_con_seq(s1: &str, s2: &str) -> String {
let mut max: Option<String> = None; // Holds value of string with maximum length
let mut current = String::new(); // String container to hold current longest value
let mut s1_iter = s1.chars().peekable(); // Peekable iterator for string s1
let mut s2_iter = s2.chars(); //Iterator for string s2
let mut s2_prev_pos = s2_iter.clone(); // Iterator that holds position of previous location of first iterator
let mut s1_prev_pos = s1_iter.clone(); // Peekable iterator used to make sure all possible combinations are located.
loop {
let s1_char = s1_iter.next(); // Get character in s1
if current.is_empty() {
// If no consecutive string found yet store location of iterator
s1_prev_pos = s1_iter.clone()
}
match s1_char {
Some(s1_char) => loop {
match s2_iter.next() {
Some(s2_char) if s1_char == s2_char => {
current.push(s1_char);
s2_prev_pos = s2_iter.clone();
break;
}
Some(_) => continue,
None => {
s2_iter = s2_prev_pos.clone();
break;
}
}
},
None => match s1_prev_pos.peek() {
Some(_) => {
match &max {
Some(max_str) => {
if max_str.len() < current.len() {
max = Some(current.clone());
}
current.clear();
}
None => {
max = Some(current.clone());
current.clear();
}
}
s1_iter = s1_prev_pos.clone();
s2_iter = s2.chars();
}
None => break,
},
}
}
max.unwrap_or_default()
}
fn criterion_benchmark(c: &mut Criterion) {
let s1 = "GXTKAYB";
let s2 = "AGGTAB";
c.bench_function("Benchmark", move |b| b.iter(|| longest_con_seq(s1, s2)));
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
#[test]
fn example_1() {
assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD");
}
#[test]
fn example_2() {
assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB");
}
#[test]
fn example_3() {
assert_eq!(longest_con_seq("aaaa", "aa"), "aa");
}
#[test]
fn example_4() {
assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA");
}
Run Rustfmt, a tool for automatically formatting Rust code to the community-accepted style.
Run Clippy, a tool for finding common mistakes that may not be compilation errors but are unlikely to be what the programmer intended.
Don't accept a
&String
— Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument?Don't use explicit
return
keywords at the end of blocks.Instead of
if let Some(_) = max { max.unwrap()
bind the variable:
if let Some(x) = max { x
Your whole last conditional can be simplified to
max.unwrap_or_default()
.Add some automated tests.
#[test] fn example_1() { assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD"); } #[test] fn example_2() { assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB"); } #[test] fn example_3() { assert_eq!(longest_con_seq("aaaa", "aa"), "aa"); } #[test] fn example_4() { assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA"); }
As before, don't match on
Some
and thenunwrap
, bind the variable. We can also avoid the clone by matching on a reference:match max { Some(_) => { let max_str = max.clone(); let max_str = max_str.unwrap();
match &max { Some(max_str) => {
current.clear()
is used in both match arms; extract it.max = Some(current.clone())
is the same; extract it and use a boolean.let do_it = match &max { Some(max_str) => { max_str.len() < current.len() } None => { true } }; if do_it { max = Some(current.clone()); }
This construct can be replaced by
as_ref
andmap_or
:if max.as_ref().map_or(true, |s| s.len() < current.len()) { max = Some(current.clone()); }
#[macro_use]
extern crate criterion;
use criterion::Criterion;
fn longest_con_seq(s1: &str, s2: &str) -> String {
let mut max: Option<String> = None; // Holds value of string with maximum length
let mut current = String::new(); // String container to hold current longest value
let mut s1_iter = s1.chars().peekable(); // Peekable iterator for string s1
let mut s2_iter = s2.chars(); //Iterator for string s2
let mut s2_prev_pos = s2_iter.clone(); // Iterator that holds position of previous location of first iterator
let mut s1_prev_pos = s1_iter.clone(); // Peekable iterator used to make sure all possible combinations are located.
loop {
let s1_char = s1_iter.next(); // Get character in s1
if current.is_empty() {
// If no consecutive string found yet store location of iterator
s1_prev_pos = s1_iter.clone()
}
match s1_char {
Some(s1_char) => loop {
match s2_iter.next() {
Some(s2_char) if s1_char == s2_char => {
current.push(s1_char);
s2_prev_pos = s2_iter.clone();
break;
}
Some(_) => continue,
None => {
s2_iter = s2_prev_pos.clone();
break;
}
}
},
None => match s1_prev_pos.peek() {
Some(_) => {
if max.as_ref().map_or(true, |s| s.len() < current.len()) {
max = Some(current.clone());
}
current.clear();
s1_iter = s1_prev_pos.clone();
s2_iter = s2.chars();
}
None => break,
},
}
}
max.unwrap_or_default()
}
fn criterion_benchmark(c: &mut Criterion) {
let s1 = "GXTKAYB";
let s2 = "AGGTAB";
c.bench_function("Benchmark", move |b| b.iter(|| longest_con_seq(s1, s2)));
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
#[test]
fn example_1() {
assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD");
}
#[test]
fn example_2() {
assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB");
}
#[test]
fn example_3() {
assert_eq!(longest_con_seq("aaaa", "aa"), "aa");
}
#[test]
fn example_4() {
assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA");
}
With numerous copies of iterators made and copies of Strings being made
Copies of iterators are generally going to be extremely lightweight. In many cases, they are equivalent to a pointer and and offset.
Cloning a String
is slightly less ideal, but that may just be a restriction of the algorithm.
Run Rustfmt, a tool for automatically formatting Rust code to the community-accepted style.
Run Clippy, a tool for finding common mistakes that may not be compilation errors but are unlikely to be what the programmer intended.
Don't accept a
&String
— Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument?Don't use explicit
return
keywords at the end of blocks.Instead of
if let Some(_) = max { max.unwrap()
bind the variable:
if let Some(x) = max { x
Your whole last conditional can be simplified to
max.unwrap_or_default()
.Add some automated tests.
#[test] fn example_1() { assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD"); } #[test] fn example_2() { assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB"); } #[test] fn example_3() { assert_eq!(longest_con_seq("aaaa", "aa"), "aa"); } #[test] fn example_4() { assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA"); }
As before, don't match on
Some
and thenunwrap
, bind the variable. We can also avoid the clone by matching on a reference:match max { Some(_) => { let max_str = max.clone(); let max_str = max_str.unwrap();
match &max { Some(max_str) => {
End result
#[macro_use]
extern crate criterion;
use criterion::Criterion;
fn longest_con_seq(s1: &str, s2: &str) -> String {
let mut max: Option<String> = None; // Holds value of string with maximum length
let mut current = String::new(); // String container to hold current longest value
let mut s1_iter = s1.chars().peekable(); // Peekable iterator for string s1
let mut s2_iter = s2.chars(); //Iterator for string s2
let mut s2_prev_pos = s2_iter.clone(); // Iterator that holds position of previous location of first iterator
let mut s1_prev_pos = s1_iter.clone(); // Peekable iterator used to make sure all possible combinations are located.
loop {
let s1_char = s1_iter.next(); // Get character in s1
if current.is_empty() {
// If no consequtiveconsecutive string found yet store location of iterator
s1_prev_pos = s1_iter.clone()
}
match s1_char {
Some(s1_char) => loop {
match s2_iter.next() {
Some(s2_char) if s1_char == s2_char => {
current.push(s1_char);
s2_prev_pos = s2_iter.clone();
break;
}
Some(_) => continue,
None => {
s2_iter = s2_prev_pos.clone();
break;
}
}
},
None => match s1_prev_pos.peek() {
Some(_) => {
match max&max {
Some(_max_str) => {
let max_str = max.clone();
let max_str = max_str.unwrap();
if max_str.len() < current.len() {
max = Some(current.clone());
}
current.clear();
}
None => {
max = Some(current.clone());
current.clear();
}
}
s1_iter = s1_prev_pos.clone();
s2_iter = s2.chars();
}
None => break,
},
}
}
max.unwrap_or_default()
}
fn criterion_benchmark(c: &mut Criterion) {
let s1 = "GXTKAYB";
let s2 = "AGGTAB";
c.bench_function("Benchmark", move |b| b.iter(|| longest_con_seq(s1, s2)));
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
#[test]
fn example_1() {
assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD");
}
#[test]
fn example_2() {
assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB");
}
#[test]
fn example_3() {
assert_eq!(longest_con_seq("aaaa", "aa"), "aa");
}
#[test]
fn example_4() {
assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA");
}
Run Rustfmt, a tool for automatically formatting Rust code to the community-accepted style.
Run Clippy, a tool for finding common mistakes that may not be compilation errors but are unlikely to be what the programmer intended.
Don't accept a
&String
— Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument?Don't use explicit
return
keywords at the end of blocks.Instead of
if let Some(_) = max { max.unwrap()
bind the variable:
if let Some(x) = max { x
Your whole last conditional can be simplified to
max.unwrap_or_default()
.
#[macro_use]
extern crate criterion;
use criterion::Criterion;
fn longest_con_seq(s1: &str, s2: &str) -> String {
let mut max: Option<String> = None; // Holds value of string with maximum length
let mut current = String::new(); // String container to hold current longest value
let mut s1_iter = s1.chars().peekable(); // Peekable iterator for string s1
let mut s2_iter = s2.chars(); //Iterator for string s2
let mut s2_prev_pos = s2_iter.clone(); // Iterator that holds position of previous location of first iterator
let mut s1_prev_pos = s1_iter.clone(); // Peekable iterator used to make sure all possible combinations are located.
loop {
let s1_char = s1_iter.next(); // Get character in s1
if current.is_empty() {
// If no consequtive string found yet store location of iterator
s1_prev_pos = s1_iter.clone()
}
match s1_char {
Some(s1_char) => loop {
match s2_iter.next() {
Some(s2_char) if s1_char == s2_char => {
current.push(s1_char);
s2_prev_pos = s2_iter.clone();
break;
}
Some(_) => continue,
None => {
s2_iter = s2_prev_pos.clone();
break;
}
}
},
None => match s1_prev_pos.peek() {
Some(_) => {
match max {
Some(_) => {
let max_str = max.clone();
let max_str = max_str.unwrap();
if max_str.len() < current.len() {
max = Some(current.clone());
}
current.clear();
}
None => {
max = Some(current.clone());
current.clear();
}
}
s1_iter = s1_prev_pos.clone();
s2_iter = s2.chars();
}
None => break,
},
}
}
max.unwrap_or_default()
}
fn criterion_benchmark(c: &mut Criterion) {
let s1 = "GXTKAYB";
let s2 = "AGGTAB";
c.bench_function("Benchmark", move |b| b.iter(|| longest_con_seq(s1, s2)));
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
Run Rustfmt, a tool for automatically formatting Rust code to the community-accepted style.
Run Clippy, a tool for finding common mistakes that may not be compilation errors but are unlikely to be what the programmer intended.
Don't accept a
&String
— Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument?Don't use explicit
return
keywords at the end of blocks.Instead of
if let Some(_) = max { max.unwrap()
bind the variable:
if let Some(x) = max { x
Your whole last conditional can be simplified to
max.unwrap_or_default()
.Add some automated tests.
#[test] fn example_1() { assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD"); } #[test] fn example_2() { assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB"); } #[test] fn example_3() { assert_eq!(longest_con_seq("aaaa", "aa"), "aa"); } #[test] fn example_4() { assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA"); }
As before, don't match on
Some
and thenunwrap
, bind the variable. We can also avoid the clone by matching on a reference:match max { Some(_) => { let max_str = max.clone(); let max_str = max_str.unwrap();
match &max { Some(max_str) => {
End result
#[macro_use]
extern crate criterion;
use criterion::Criterion;
fn longest_con_seq(s1: &str, s2: &str) -> String {
let mut max: Option<String> = None; // Holds value of string with maximum length
let mut current = String::new(); // String container to hold current longest value
let mut s1_iter = s1.chars().peekable(); // Peekable iterator for string s1
let mut s2_iter = s2.chars(); //Iterator for string s2
let mut s2_prev_pos = s2_iter.clone(); // Iterator that holds position of previous location of first iterator
let mut s1_prev_pos = s1_iter.clone(); // Peekable iterator used to make sure all possible combinations are located.
loop {
let s1_char = s1_iter.next(); // Get character in s1
if current.is_empty() {
// If no consecutive string found yet store location of iterator
s1_prev_pos = s1_iter.clone()
}
match s1_char {
Some(s1_char) => loop {
match s2_iter.next() {
Some(s2_char) if s1_char == s2_char => {
current.push(s1_char);
s2_prev_pos = s2_iter.clone();
break;
}
Some(_) => continue,
None => {
s2_iter = s2_prev_pos.clone();
break;
}
}
},
None => match s1_prev_pos.peek() {
Some(_) => {
match &max {
Some(max_str) => {
if max_str.len() < current.len() {
max = Some(current.clone());
}
current.clear();
}
None => {
max = Some(current.clone());
current.clear();
}
}
s1_iter = s1_prev_pos.clone();
s2_iter = s2.chars();
}
None => break,
},
}
}
max.unwrap_or_default()
}
fn criterion_benchmark(c: &mut Criterion) {
let s1 = "GXTKAYB";
let s2 = "AGGTAB";
c.bench_function("Benchmark", move |b| b.iter(|| longest_con_seq(s1, s2)));
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
#[test]
fn example_1() {
assert_eq!(longest_con_seq("ABAZDC", "BACBAD"), "ABAD");
}
#[test]
fn example_2() {
assert_eq!(longest_con_seq("AGGTAB", "GXTKAYB"), "GTAB");
}
#[test]
fn example_3() {
assert_eq!(longest_con_seq("aaaa", "aa"), "aa");
}
#[test]
fn example_4() {
assert_eq!(longest_con_seq("ABBA", "ABJABA"), "ABBA");
}
Run Rustfmt, a tool for automatically formatting Rust code to the community-accepted style.
Run Clippy, a tool for finding common mistakes that may not be compilation errors but are unlikely to be what the programmer intended.
Don't accept a
&String
— Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument?Don't use explicit
return
keywords at the end of blocks.Instead of
if let Some(_) = max { max.unwrap()
bind the variable:
if let Some(x) = max { x
Your whole last conditional can be simplified to
max.unwrap_or_default()
.
#[macro_use]
extern crate criterion;
use criterion::Criterion;
fn longest_con_seq(s1: &str, s2: &str) -> String {
let mut max: Option<String> = None; // Holds value of string with maximum length
let mut current = String::new(); // String container to hold current longest value
let mut s1_iter = s1.chars().peekable(); // Peekable iterator for string s1
let mut s2_iter = s2.chars(); //Iterator for string s2
let mut s2_prev_pos = s2_iter.clone(); // Iterator that holds position of previous location of first iterator
let mut s1_prev_pos = s1_iter.clone(); // Peekable iterator used to make sure all possible combinations are located.
loop {
let s1_char = s1_iter.next(); // Get character in s1
if current.is_empty() {
// If no consequtive string found yet store location of iterator
s1_prev_pos = s1_iter.clone()
}
match s1_char {
Some(s1_char) => loop {
match s2_iter.next() {
Some(s2_char) if s1_char == s2_char => {
current.push(s1_char);
s2_prev_pos = s2_iter.clone();
break;
}
Some(_) => continue,
None => {
s2_iter = s2_prev_pos.clone();
break;
}
}
},
None => match s1_prev_pos.peek() {
Some(_) => {
match max {
Some(_) => {
let max_str = max.clone();
let max_str = max_str.unwrap();
if max_str.len() < current.len() {
max = Some(current.clone());
}
current.clear();
}
None => {
max = Some(current.clone());
current.clear();
}
}
s1_iter = s1_prev_pos.clone();
s2_iter = s2.chars();
}
None => break,
},
}
}
max.unwrap_or_default()
}
fn criterion_benchmark(c: &mut Criterion) {
let s1 = "GXTKAYB";
let s2 = "AGGTAB";
c.bench_function("Benchmark", move |b| b.iter(|| longest_con_seq(s1, s2)));
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);