-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disentangled arrays in iteration: repl, serialization and shared arrays #15463
Conversation
@@ -116,16 +114,16 @@ function serialize(s::SerializationState, t::Tuple) | |||
writetag(s.io, LONGTUPLE_TAG) | |||
write(s.io, Int32(l)) | |||
end | |||
for i = 1:l | |||
serialize(s, t[i]) | |||
for e in t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e
for "element" (of array).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not a great variable name, since it's also a constant 2.718...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I haven't thought about it. Would you suggest another name which is normally used for elements of a vector in Julia core? (I think I saw x
a couple of times)
Since this PR is already merged, I'm going to include the change into the next one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for elem in t
maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't x
more common? elem
is quite verbose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's 4 characters... this one-letter habit is harmful, variable names that describe what's going on would be a big improvement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for elem in
appears only once in the code while for x in
is used a lot. Once people get used to a convention, it can work even if it's not very explicit.
Anyway, this example is so short that either solution is perfectly fine IMHO.
Disentangled arrays in iteration: repl, serialization and shared arrays
Many thanks! |
As suggested in #15434, replaced occurrences of
for i=1:length(A)
by eitherfor a in A
orfor i in eachindex(A)
for flexibility and speed.I omitted 2 kinds of cases:
for i=1:n
wheren
comes from outside (e.g. length of array being deserialized) or is used in function logic anyway (seedeserialize(s::SerializationState, t::DataType)
for example);for i=2:length(A)
).Let me know if there's a way to improve these 2 cases as well.
cc: @timholy