Skip to content
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

swap! #3

Merged
merged 1 commit into from
Sep 28, 2015
Merged

swap! #3

merged 1 commit into from
Sep 28, 2015

Conversation

diegozea
Copy link
Contributor

This adds a swap! function for interchange elements on the IndexedArray

@test findfirst(original, "dog") == 2
end


Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Can you switch to a four-space indent and also get rid of the added newline at the end of the file?

I think it would also be good to have a test for when to == from. Perhaps the function itself should special-case for this and return immediately as well.

@diegozea
Copy link
Contributor Author

Sure ;) You're welcome!

export AbstractIndexedArray, IndexedArray, IndexedArrayError, findfirst!
"`swap!(ia::IndexedArray, to::Int, from::Int)` interchange/swap the values on the indices `to` and `from` in the `IndexedArray`"
function swap!(ia::IndexedArray, to::Int, from::Int)
if to == from && checkbounds(ia,to)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I would have put the checkbounds as the first line inside the if block, not as part of the conditional, which is how it is typically done. It is interesting that the current code also works correctly though, which has led me to file JuliaLang/julia#13334

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now it's outside the the conditional.

@garrison
Copy link
Owner

Can you squash to a single commit please?

swapgit add .o == from

+ checkbounds

Update IndexedArrays.jl
@diegozea
Copy link
Contributor Author

Squashed ;)

garrison added a commit that referenced this pull request Sep 28, 2015
@garrison garrison merged commit 1bfc7f3 into garrison:master Sep 28, 2015
@garrison
Copy link
Owner

Thanks!

@diegozea
Copy link
Contributor Author

You're welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants