How can I improve this code?

[underthetabledowg]

The first option is the commented code above. Not cool, I know.

Then I thought of this second way but still not sure if it looks good. What do you think? Any tips are appreciated.

[firebits.io]

Pattern matching using tuples of booleans is a much better approach since it provides you with exhaustiveness check.

You might want to use chars instead of strings, though:

time.contains('s')

As for how can you improve the code, there's not much else to say given what you are showing us. If you want more/better feedback, then you should provide us with more information on the design/intent of the code in question.

[jumpnbrownweasel]

Please post text, not images -- not everyone can read the images.
It's very easy to format your code:

Forum Code Formatting and Syntax Highlighting

To format code in this forum you need to surround the code with three backticks (```). For example, typing this... ``` fn main() { println!() } ``` ...will be rendered as highlighted Rust code, like so: fn main() { println!() } You can also tell the forum software which language you're using by adding the language after the opening backticks. ```cpp #include <iostream> int main() { std::cout << "Hello World!"; } ``` Will appear with highlighting as: #include <iostream> int main(…

[underthetabledowg]

I often use images because of the graphical resources like drawing and stuff... But sure thing, thank you for the advice

[Jesper]

You don't need the default clause - since you cover all the combinations. Assuming the commented code is the reference, 4 clauses will do...

match (time.contains('s'), time.contains('m'), time.contains('h')) {
    (true, true, true) => println!("seconds, minutes, and hours."),
    (true, true, false) => println!("seconds and minutes."),
    (true, false, false) => println!("only seconds."),
    _ => println!("no time."),
}

[underthetabledowg]

not really, as I want to handle it even when I have "h" but not "s", but you are right in a normal situation

[akrauze]

Maybe you should write your own parser, that would report more detailed information than "time contains a given letter". This would probably be more performant too, since now you scan whole string 3 times, but you probably could write a parser that would only need to traverse it once.

[MOCKBA]

It's a very good idea and since we generate ideas as a generative AI , you can also consider a 'push' approach when during the parsing you recognize some pattern, you can immediately push it for processing.

[kingwingfly]

May be use only one for loop on chars to check s m h. Use 3 bits of u8 to record the result. With matching branches 0b111 0b110 0b100 ..., it may looks more graceful.

Or maybe you can simply use a Finite State Machine.

[danjl1100]

Another take on matching the "look" of the original commented code:

match (time.contains('s'), time.contains('m'), time.contains('h')) {
    (true, true, true) => println!("seconds, minutes, and hours."),
    (true, true, _) => println!("seconds and minutes."),
    (true, _, _) => println!("only seconds."),
    _ => println!("no time."),
}

This loses some perks of exhaustive-ness checking (the order matters now) but to me it makes it easier to see the "all three", "first two", "first one", "none" cases at a glance. (e.g. I'm not looking at a truth table)

[Kyllingene]

@danjl1100

This loses some perks of exhaustive-ness checking (the order matters now)

While strictly speaking this is true, if you move them around, you'll get a warning about unreachable arms. Not as good as an error, but at least it's something.

[OGulsan]

@firebits.io

You might want to use chars instead of strings, though:

time.contains('s')

May I ask why? Is there a performance difference?

[cod10129]

Yes, there is.
str::contains::<char> eventually calls <std::str::pattern::CharSearcher as Pattern>::next_match (implementation), which uses memchr, a fast function to search for a single byte in a string.

str::contains::<&str> uses std::str::pattern::StrSearcher::next_match (implementation), which uses an algorithm for searching for a string, which has to support searching for strings of more than one character, and is thus slower.

I compared the generated assembly of the two functions, and I found something interesting. The optimizer can effectively see through the StrSearcher and generates similar assembly for both functions, each using one memchr call.

contains_char

contains_char:
        mov     rdx, rsi
        mov     rsi, rdi
        cmp     rdx, 15
        ja      .LBB0_3
        test    rdx, rdx
        je      .LBB0_2
        dec     rdx
        xor     ecx, ecx
.LBB0_5:
        cmp     byte ptr [rsi + rcx], 97
        sete    al
        je      .LBB0_7
        lea     rdi, [rcx + 1]
        cmp     rdx, rcx
        mov     rcx, rdi
        jne     .LBB0_5
.LBB0_7:
        ret
.LBB0_3:
        push    rax
        mov     edi, 97
        call    qword ptr [rip + core::slice::memchr::memchr_aligned::hf6aa57362cec11a2@GOTPCREL]
        cmp     rax, 1
        sete    al
        add     rsp, 8
        ret
.LBB0_2:
        xor     eax, eax
        ret

contains_str

contains_str:
        mov     rdx, rsi
        mov     rsi, rdi
        cmp     rdx, 1
        jbe     .LBB1_1
        cmp     rdx, 15
        ja      .LBB1_9
        dec     rdx
        xor     ecx, ecx
.LBB1_6:
        cmp     byte ptr [rsi + rcx], 97
        sete    al
        je      .LBB1_8
        lea     rdi, [rcx + 1]
        cmp     rdx, rcx
        mov     rcx, rdi
        jne     .LBB1_6
.LBB1_8:
        ret
.LBB1_1:
        jne     .LBB1_2
        cmp     byte ptr [rsi], 97
        sete    al
        ret
.LBB1_9:
        push    rax
        mov     edi, 97
        call    qword ptr [rip + core::slice::memchr::memchr_aligned::hf6aa57362cec11a2@GOTPCREL]
        cmp     rax, 1
        sete    al
        add     rsp, 8
        ret
.LBB1_2:
        xor     eax, eax
        ret

Compiler Explorer - Rust (rustc 1.83.0)

However, this optimization is not something to rely on. There are also other reasons to provide chars to string searching methods when possible^[1]. For instance, "string".split("A").rev() (note the argument is a &str) doesn't compile, because as the DoubleEndedSearcher docs put it:

(&str)::Searcher is not a DoubleEndedSearcher because the pattern "aa" in the haystack "aaa" matches as either "[aa]a" or "a[aa]", depending on which side it is searched.

In this case, you can use .split('A')^[2] to obtain a reversible iterator (and as a bonus, it optimizes better).

---

1. though not in the specific case of contains ↩︎

2. note the single quotes ↩︎

[OGulsan]

thanks! i learned something new

[joshka]

On the assumption that this is the start of some code used for parsing a duration (guessing here), then the best improvement is to not write this code yourself. Use a library for parsing time from strings into times instead. E.g. jiff::fmt::friendly - Rust