Wrote some code, but I think it is quite bad.

[dreamy sedge]

This code works and does its job well, the problem is that I am still learning Rust (just finished the strings in rustlings) and wanted to do something others were doing in our CS class in Java. I still lack the knowledge and just general reflexes, and hope that someone will give some tips on how to reduce/optimise this code.

The goal of the code is to just turn a signed integer and change it into two's compliment binary notation.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=9cf51aaea7c5a824f78d0d2ffa41745b

[left dock]

first thing i'd change is to change the signature of your functions to take i32 by value, not by reference

[cursive matrix]

Most of this code already exists in the standard library, as part of the {:b} formatting specifier

I'll be ignoring that going forward: I assume this was a learning exercise

[dreamy sedge]

It is, fensjfneh

[dreamy sedge]

whuh? (i do not understand what you mean)

[left dock]

fn int_to_twos_binary(int: &i32) -> String should just be fn int_to_twos_binary(int: i32) -> String

i.e. i32 instead of &i32

if you're on a 64 bit system, the pointer to the i32 is bigger than the i32 itself 🙂

[dreamy sedge]

mhm although I do not know if I want to change the value of variable itself, at least from what i understood, doing this wouldn't change the original variable

[left dock]

it won't either way

&i32 is an immutable reference

[cursive matrix]

I have several suggestions 😄

• If a type is Copy and smaller than a reference (which is usually 8 bytes), take it by value rather than by &: using a reference is actually slower.
• It's a bit more common to use literal suffixes rather than type annotations: let basic_number = 4u32;, let signed_number = -1023i32; Add underscores for readability if you prefer: -1023_i32 is a valid integer literal.
• "".to_owned() is a String even if you don't use a type annotation. In general, type annotations are rare in idiomatic code.
• result = "0".to_owned() + &result is an extremely inefficient way to build a string in a loop. You end up allocating a ton of intermediate strings, and even if you used insert(0) to avoid that, you'd be copying the entire string every time. I'd suggest building a Vec<u8> of 0 and 1, reverseing it, and then turning that into a string at the end (which can be done nicely with fold)
• _other => {} is an unreachable arm. It's fine as it is, but I'd probably write it as _ => unreachable!(). Or switch to an if, matching on the result of % is annoying since the compiler can't tell there's lots of unreachable values.
• int_to_twos_binary can be implemented much more efficiently (and with less effort, too) if you use bitwise operations for it rather than rewriting the string entirely for negative inputs. For some hints, ||1 << (i32::BITS - 1) returns a bitmask with the highest bit set and nothing else||, ||int & bitmask != 0 returns whether the bit represented by the mask is set||, and ||(bitmask as u32 >> 1) as i32 shifts the set bit one to the right. You do need the double cast, else you get the wrong kind of shift||. I'll ignore this reimplementation suggestion in the later points. Also, this idea also works for int_to_binary, actually.

• each replace makes a whole new string. Replacing three times in a row creates two intermediate strings. We can do better by doing the replacement by hand (which also removes the need for the temporary 2 value). The easiest way to do that would be chars and map (see below)
• You could slightly abbreviate string_change and string_no_change by using split_at: result.split_at(one_stop as usize)
• Technically you could also replace the as usize by merging it with the check: if let Ok(one_stop) = usize::try_from(one_stop) { ... }
• You don't need to keep adding increasing integer suffixes to your variables (string_change, string_change1, etc). Shadowing exists and is idiomatic.

And here is the way to avoid the triple replace:
||```rs
let result = result
.chars()
.map(|c| match c {
'0' => '1',
'1' => '0',
c => c,
})
.collect::<String>();

[dreamy sedge]

Thank you very much :3
Will look into everything you said and research into how it works to get a hang of it, thank you a lot again <3

[dreamy sedge]

I know it has been a while but I have a question about fold.
How do I use it? I can't even find any resources that explain the concept to me, but maybe I just do not understand iterators well enough yet.

[ivory shale]

fold is a way of reducing an iterator to a value, with a function that processes one element at a time. It's quite flexible; some possible uses are finding the sum/product of everything in a collection, or creating a new collection. fold is given an initial value (of whatever type you want), and a function that takes that value and an element of the iterator, and returns a value of the same type as the initial value, which is passed to the next call to the function (and so on). There's some docs here if you haven't seen them: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.fold. Feel free to ask further questions; I'm not sure exactly what part you're missing.

That said, fold isn't necessary for this use case. You can, for instance, rewrite int_to_binary as:

fn int_to_binary(int: &u32) -> String {
    let mut result: Vec<u8> = vec![];
    let mut number: u32 = *int;
    while number > 0 {
        match number % 2 {
            0 => {
                result.push('0' as u8);
            }
            1 => {
                result.push('1' as u8);
                number -= 1;
            }
            _other => {}
        }
        number = number / 2;
    }
    result.into_iter().map(|x| x as char).rev().collect()
}

(It's also possible to use Vec<char> instead of Vec<u8> but I'm not sure whether that would perform worse. It probably doesn't make much difference either way.)

[ivory shale]

I also have more feedback on the code, if you want it. But I'd prefer if you apply many of the previous suggestions first so I don't end up giving duplicate suggestions.

[cursive matrix]

iter.fold(init, closure)
```is equivalent to this
```rs
let mut acc = init; //acc = accumulator
for elem in iter {
  acc = closure(acc, elem);
}
acc
```For some examples:
- `fold(0, |acc, elem| acc + elem)` will sum a list of numbers 
- `fold(1, |acc, elem| acc * elem)` will multiply one
- `fold(Vec::new(), |acc, elem| { acc.push(elem); acc})` is equivalent to `collect::<Vec<_>>()`.

In this case, the suggestion was to use a fold to convert a `Vec<u8>` of 0s and 1s into a string. I'm not sure _why_ I suggested that, this is easier with the `map().collect()` I explained later, but you _could_ use a fold for this:
```rs
.fold(String::new(), |acc, elem| {acc.push(elem + b'0' as char); acc})

@dreamy sedge