MathNya
Suggestion: Consider not using references for all numeric function parameters
Hey again,
I noticed that various functions take &u32s as arguments:
e.g.
pub fn insert_new_row(&mut self, row_index: &u32, num_rows: &u32)
This is actually likely less efficient and also less ergonomic than just taking in u32s because as it stands today, the pointer to the u32 would need to be copied instead (which is typically 64-bits / 8 bytes) while creating a copy of the number itself would only take 32-bits / 4 bytes.
I think this is why you will see that the Copy trait is implemented for such primitive types in Rust, as it is often as efficient or more efficient to just take a copy of the value than a copy of the pointer reference.
Just a thought, not sure if I'm missing the reason why references were used for these cases.
Cheers Fotis
Yeah, this always seemed weird and unergonomic to me.
Also, get_ prefixes are uncommon in Rust, they only add noise IMO.
I have since gotten used to it since changing this is likely to be a HUGE code-breaking exercise...
Thank you for pointing this out. I did not understand the specifications regarding copying.
However, I would have to raise a major version to change this. We just recently went to 2.0.0 and will wait a while before considering a response.
Also, get_ prefixes are uncommon in Rust, they only add noise IMO.
I did not know this either. I usually use PHP so get_ was a given.
I will consider this too when I do a major version in a while.
Also, get_ prefixes are uncommon in Rust, they only add noise IMO.
I did not know this either. I usually use PHP so get_ was a given.
I will consider this too when I do a major version in a while.
Honestly, it isn't idiomatic Rust style, but I'm OK with it. Many languages have this syntax, like JavaScript's obj.getMyTeaCup(), C#'s obj.GetMyTeaCup() or obj.MyTeaCup and Rust sometimes has obj.get_my_tea_cup() instead of obj.my_tea_cup()...
But in Rust, usually the modifier get_ is used when some work needs to be done in order to return some info, while the syntax without get_ is used when it is a trivial reference, like fn my_tea_cup(&self) -> &TeaCup { self.my_tea_cup }... which is usually coupled with #[inline(always)] to note that fact.
I think the reference parameters like &i32 are way more annoying that those get_'s...
For the coming version, do you also think about reducing panic in codebase?
@piaoger, could you open a separate issue with details about panic conditions? This issue is about API style
@piaoger, could you open a separate issue with details about panic conditions? This issue is about API style
I am mainly talking about error handling style in this crate. Just made quick search in code, and there are 237 matches across 215 files.
In my attempt to reduce the memory usage of this crate, I find that the liberal usage of returning references to primitive data types is preventing most venues of compacting data size because simply those values must be there to reference.
For example, I cannot replace numerous boolean flags with a single bitflag because the return type is &bool. Frustrating to say the least.
For the coming version, do you also think about reducing panic in codebase?
It seems v3 is coming, is it also a chance to reduce panic usage? @MathNya
@piaoger Yes, we are working on a new version of the program. Version 3.0 allows all kinds of changes. Of course, this includes destructive changes. If you are going to make any major changes, now is the time.