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

Assignment bugfix #4

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Assignment bugfix #4

wants to merge 5 commits into from

Conversation

CasBex
Copy link
Owner

@CasBex CasBex commented Jul 13, 2024

I implemented an in-place and out-of-place version of one of the simple sorting algorithms.

There's the following bugs/problems that I can spot right out-of-the-box

  1. mysort doesn't pass reverse to mysort!
  2. A reverse sort for AbstractArray{T} could fail due to the non-existence of (-)(::T) (e.g. arrays of strings)
  3. A reverse in-place sort is not actually mutating and has a return value
  4. While mysort is type-stable, the return type can be weird. For example mysort(::String) -> Vector{Char}
  5. mysort! will fail for arrays with weird indices (e.g. OffsetArrays)

Steps for completing this assignment are as follows:

  • Mentee files an issue with FixMe.jl
  • Mentee fixes these problems
  • Mentee writes some tests and uses these to show that their fix is working
  • Mentee makes a PR to FixMe.jl
  • Mentor does code review, prompts the mentee to make some tests and asks guiding questions when the mentee doesn't spot some of the above problems.
  • Mentor merges PR to his fork

Before merging we should at least take this for a test drive. I'm looking for volunteer mentors/mentees.
If it's approved after the test-drive (or after some iteration) the README should be updated with instructions.

  • Test drive the assignment
  • Add to README

src/FixMe.jl Outdated Show resolved Hide resolved
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