odin-lang / Odin

Odin Programming Language
https://odin-lang.org
BSD 3-Clause "New" or "Revised" License
6.1k stars 550 forks source link

Add strings.builder_replace #3804

Closed Beefster09 closed 6 days ago

Beefster09 commented 1 week ago

In some cases, it can be preferable to replace substrings in a builder in-place. While this is less efficient than generating a new string in many cases, when the lengths of the strings are the same, it may sometimes be faster or the intent may be clearer.

Kelimion commented 6 days ago

Can you motivate this addition with a practical use case?

Why, when building a string, would I write "hello" 10 or 20 times and then go back to replace them all with "hellope". Why wouldn't I write "hellope" to begin with?

As an example, I could write <placeholder beefster> and then replace that with Beefster <beefster email> in a git log after the fact so I don't have to make that lookup while building the string, but I could do one pass over the git log to get all committer names and only then build out the log. Or I could finalize the string with the placeholders and then do a string.replace.

This new call feels out of place with how the string builder is used, and I'm also unclear about a solid use case where this would be preferable over waiting for the needle to be known instead of replacing it, or replacing it after the string is built.

I appreciate the effort you went through to write it and even include a test, but I'm just left wondering "why" and "when".

gingerBill commented 6 days ago

I am not going to accept this. It's kind of just enabling really bad practices from GC languages.

Also, you have clearly not kept to the coding style of the rest of the core library.

Beefster09 commented 6 days ago

The use case that motivated this was replacing the underscores in an enum string.

I'm not going to argue further for its inclusion, but I am wondering where exactly I deviated from the core coding style so that I can know for next time.

gingerBill commented 6 days ago

@Beefster09 If it's an enum string, that means it is a compile time variable, and probably just want to call one of the strings.to_*_case calls.

Kelimion commented 6 days ago

Or, and I'm just throwing this out there: Bake it into an enumerated array.

    Foo :: enum {
        A_lot_of_dashes,
    }

    FOO_STRING :: [Foo]string{
        .A_lot_of_dashes = "A lot of dashes",
    }

    fmt.println(FOO_STRING[.A_lot_of_dashes])