Skip to content

Added BogoSort in Swift 4.1 #161

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

Merged
merged 17 commits into from
Jun 29, 2018
Merged

Added BogoSort in Swift 4.1 #161

merged 17 commits into from
Jun 29, 2018

Conversation

Wesley-Arrington
Copy link
Contributor

@Wesley-Arrington Wesley-Arrington commented Jun 29, 2018

Added BogoSort in Swift 4.1

@Butt4cak3
Copy link
Contributor

Welcome @CDsigma! Thanks for the contribution!

You're the first person to submit Swift code to the Algorithm Archive. You correctly added the code import to the .md file, but currently it doesn't know what lang="swift" means. For that, you have to go into book.json (which is at the root of this repository), scroll down to where all the programming languages are listed and add Swift like so:

				...
				{
					"lang": "go",
					"name": "Go"
				},
				{
					"lang": "swift",
					"name": "Swift"
				}

I don't know if we have a Swift programmer to review your code, but we will see what we can do. Your code looks pretty good, so I'm sure it will get merged soon.

If you want, you can also add your name to the end of the CONTRIBUTORS.md file, but you don't have to.

@Butt4cak3 Butt4cak3 added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label Jun 29, 2018
@Butt4cak3
Copy link
Contributor

Perfect. I totally didn't catch that you only changed lang:"swift" but not the other "lang="swift" attribute, but now it should be good. Now to wait for a Swift developer to show up. If we can't find one, I'll try to get your PR merged tomorrow or the day after.

@Wesley-Arrington
Copy link
Contributor Author

Wesley-Arrington commented Jun 29, 2018

Awesome :)

Thanks for your help 😊

@zsparal
Copy link
Contributor

zsparal commented Jun 29, 2018

One small note: you commited with a git user/email different from your GitHub one. Was this intentional? If not, feel free to fix it and force push your changes.



func isSorted(inputArray: [Int]) -> Bool {

Copy link
Member

@jiegillet jiegillet Jun 29, 2018

Choose a reason for hiding this comment

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

Nitpick 1: 2 out of 3 functions start with an empty line. I don't know which style is best, but I do know that consistency is always rule number 1 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya that's a good note I'll make the line spacing consistent in each function




func shuffle(inputArray: inout [Int]) -> [Int] {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick 2: a quick Google search showed me that there is a native shuffle function in Swift 4.2+. Maybe it's best to use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there's a reason to use version 4.1 specifically.

Copy link
Member

Choose a reason for hiding this comment

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

Oh my bad, I had not seen the version was specifically 4.1, it's right there in the title -_-
But yeah, I'd like to hear the reason too then :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most people who use Swift tend to use the version which comes with Xcode. Currently the stable build of Xcode uses Swift 4.1 so I thought that would be the best choice.

There is an Xcode 10 beta which uses Swift 4.2 but that won't be the stable build for a few months so until then I think Swift 4.1 is the best choice.

@Wesley-Arrington
Copy link
Contributor Author

Wesley-Arrington commented Jun 29, 2018

@Gustorn Ya I noticed that just after I committed. I didn't mean to do it but I also don't mind so I think I'll leave it 🤷

@zsparal
Copy link
Contributor

zsparal commented Jun 29, 2018

@CDsigma sure thing, I just thought I'd mention it since sometimes people accidentally commit with their work email (did it once or twice myself) and they'd rather not have that online. If you're OK with the current one it's all good.

I accidentally had messed up the location of some the brackets
This update should fix the location of the brackets
Okay now I have actually formatted the brackets correctly
@Butt4cak3
Copy link
Contributor

The squashed commit will have the right author, I think. It won't really matter in the end.

Copy link
Contributor

@Butt4cak3 Butt4cak3 left a comment

Choose a reason for hiding this comment

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

Now that you and @jiegillet figured some things out, I have some small changes that I'd like you to make. Once they're done, I think this one is ready to merge!

book.json Outdated
"lang": "swift",
"name": "Swift"
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

The two lines with the brackets are indented with a bunch of spaces and then 3 tabs. The file is tab indented. Try to make sure you only use tabs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that just committed some changes so no more spaces are used

@@ -36,6 +36,9 @@ In code, it looks something like this:
{% sample lang="rs" %}
[import, lang:"rust"](code/rust/bogosort.rs)
{% endmethod %}
{% sample lang="swift" %}
[import, lang:"swift"](code/swift/bogoSort.swift)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the capitalization of the "S" in the file name. Some languages (like Java) require capitalization to match classes and such inside the files, but I'd prefer to keep everything else lowercase.

Unless it's some kind of convention in Swift. In that case, keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just updated the file name 👍

@Butt4cak3
Copy link
Contributor

Since we all don't really know Swift, I'll just take the liberty to merge this one. The logic of the code is correct and so it shouldn't be an issue.

@Butt4cak3
Copy link
Contributor

Nevermind. I just found a mistake that breaks the build. You have to fix this before I can merge.

@@ -36,6 +36,9 @@ In code, it looks something like this:
{% sample lang="rs" %}
[import, lang:"rust"](code/rust/bogosort.rs)
{% endmethod %}
{% sample lang="swift" %}
[import, lang:"swift"](code/swift/bogosort.swift)
{% endmethod %}
Copy link
Contributor

Choose a reason for hiding this comment

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

There's only supposed to be one {% endmethod %} statement at the end. You have to remove the one in line 38.

@Butt4cak3 Butt4cak3 merged commit f344b76 into algorithm-archivists:master Jun 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants