Sorry for the title which may not be very clear, but while reading the source code of Crystal I stumbled across a pattern that is used a lot and illustrated in the following example.
def add_or_create_and_add_to_array(arg : Int32)
numbers = @numbers ||= [] of Int32
numbers << arg
end
After trying to simplify this kind of code, I realized that the purpose of the temporary variable is to avoid a compile error. @numbers is essentially Array(Int32) | Nil and therefore does not necessarily respond to << in spite of the fact that it definitely will be in this case.
I understand why this error shows up and this is just a side effect of having a type system. But since I wanted to try to avoid having the temporary variable, I did this instead:
crystal
def add_or_create_and_add_to_array(arg : Int32)
@numbers ||= [] of Int32
@numbers.as(Array(Int32)) << arg
end
On my working environment it is a tiny bit faster (1.19) at the cost of being a bit more verbose, especially if you do more than one thing with the array after this because you have to do .as(Type) each time.
My question is, what is your opinion on the subject? What is the best way to deal with these cases?
also
@numbers.not_nil! << arg
no need cast. but its a double check. in first snippet just one check for nil so in theory should be faster, but cpu optimization, prediction, etc make it hard to know. plus times around nanosecond so benchmarks not very useful.
I think I prefer the current solution.
@mig-hub What do you mean with a tiny bit faster? If there is much of a performance difference at all, I'd suspect after LLVM optimizations the local variable would actually be faster than two ivar lookups plus type assertion.
But apart from performance considerations, I would also prefer the current version as being better understandable and genreally "cleaner" code.
It's even nicer to do this:
getter numbers { [] of Int32 }
def foo(arg)
numbers << arg
end
Most patterns have an equivalent getter/setter/property equivalent (using wither a block, ? or !), and you can even use these macros for private instance variables too using private property etc.
I'll close this for now though, considering it's a style question, not an actionable issue.
It would make an issue if there was an actual significant performance difference.
@RX14 Yes yes we can close it. I like your getter example better actually and the one that feels more natural. I would use this one but I guess this pattern is used to be able to have nil instead of an empty array when it is not used at all. The first example I've found is for before_request in HTTP::Client. If an empty array is not much more than nil in terms of memory I would definitely go for that.
I was actually looking for a way that has no runtime cost at all. And @straight-shoota mentioned a type assertion so I was wrong in thinking .as(Type) had no runtime cost. I thought it was a bit like casting in C, only a compile time thing.
Out of the 2 examples I gave, I find the first one more readable as well by the way, it just felt wrong and part of me doesn't believe it is optimised by LLVM (at least on my machine), otherwise the benchmarks would be closer for both methods, no? But I may be wrong, I will investigate this.
Thank you everyone for your answers, it was really helpful.
A getter with a block means the block is lazily evaluated when the getter is accessed and the ivar is nil. So it's not creating an empty array unless needed (see docs).
@straight-shoota Amazing! I thought @RX14 meant getter numbers = [] of Int32 because his version does not compile, but I've just realise that is because you need the parentheses around numbers for it to compile: getter(numbers) { [] of Int32 }.
Thank you.
@mig-hub my bad, I should test code before I send it.
Most helpful comment
@straight-shoota Amazing! I thought @RX14 meant
getter numbers = [] of Int32because his version does not compile, but I've just realise that is because you need the parentheses aroundnumbersfor it to compile:getter(numbers) { [] of Int32 }.Thank you.