References in arguments for simple types

code review
1

I have a doubts to drop references from arguments below:

pub fn insert(
    tx: &Transaction,
    app_browser_window_tab_id: &i64,
    page_position: &i32,
    is_pinned: &bool,
    is_selected: &bool,
    is_attention: &bool,
) -> Result<usize, Error> {
    tx.execute(
        "INSERT INTO `app_browser_window_tab_item` (
            `app_browser_window_tab_id`,
            `page_position`,
            `is_pinned`,
            `is_selected`,
            `is_attention`
        ) VALUES (?, ?, ?, ?, ?)",
        [
            app_browser_window_tab_id,
//...

If these arguments does not require sync with parent values, maybe it's better to use unreferenced copies for integers and boolean types, because looks horrible imho

What do you think about, has it a sense to begin refactory?

2

I don't know what Transaction is but from all the other fields you can drop the reference and really you should, because they make the struct unnecessarily complicated.
References cannot be used to sync in the way you want because either you have an immutable reference & so you cannot mutate anyways or you have a mutable reference &mut and then it is not synchronized because there can only be one &mut at a time.
If you wanted it to synchronize you would need a smartpointer type that does this.
I think Rc<RefCell<T>> would work, where T is one of the types you currently have behind a reference.

EDIT
After reading the documentation again I think Rc<Cell<T>> would be the right type for the small values like i64 that you have.

But again, if your version with references worked then there is no syncing happening and you should just take off the references.

2 Likes
3

ah really o\ forgot about immutable variables by default in rust, thanks!

Transaction - is SQLite one, I don't plan to touch it, forgot to say.

So if we are skipping sync context, should I use copied values always, instead of references for int and boolean? Just to understand what is better for performance, because I heard that reference is always expensive operation. But wrote this code before understand that

After reading the documentation again I think Rc<Cell<T>> would be the right type for the small values like i64 that you have.

In fact, I wrote these references just to remind myself that's not editable data, referencing somewhere, like pointer. This code example is one sub-query in the transaction chain, maybe I really want shared reference just for app_browser_window_tab_id there

4

I agree about dropping the reference for small Copy types as parameters. But what struct are you referring to?

1 Like
5

References aren't expensive, exactly, but they aren't free either. They take up the space of a usize (usually 64 bits, i.e. u64/i64), so referencing anything that size or smaller that's Copy is automatically wasting space (because the reference takes more space than the value itself would). Integers (except u128/i128), booleans, and floats are all at most 64 bits.

Dereferencing it also takes a little time, though here I doubt it's that noticable. But in general, it's best not to take immutable references to small, Copy types like that.

2 Likes
6

Ups. Because of the formatting I automatically assumed I was looking at a struct definition, not at a function :joy:.

1 Like
7

What do you mean by "argumentd does not require sync with parents"? I was thinking you talked about the referenced value (the parent) being updated (synced) and this update also being reflected through the reference. But now that I know you were talking about function parameters and not struct fields I don't see how such an update would ever happen during the execution of a function. So now I am entirely confused by your question.

The answer btw stays the same. Hands of immutable references for small function parameters that implement Copy.