46
Is this FizzBuzz Swift-y?
Kinda, but it could be a lot better. Here's what I would do to fix it:
Extrapolate this code into a method, then call the method from the for loop.
func fizzbuzz(i: Int) -> String
{
// ...
}
There is a handy Swift feature called "Tuples". Tuples are groupings of values. We can use them to represent our results from the ...
32
The problem here isn't that your Swift looks like Objective-C. The problem is that, because you're developing a toy app, you've used a toy design, and now you don't like the toy design. Probably you followed a tutorial you found on the Internet (or even in Apple's docs), and most of those are for toy apps and toy designs. In those tutorials, the author ...
27
Let's start with some general remarks:
Swift does not require semicolons after statements (but they are allowed). From what I have seen since Swift was introduced last year,
most people do not write semicolons in Swift.
Classes are reference types, and the properties of an instance of a class
can be modified even if the instance is declares as a constant ...
25
You could simplify the above significantly:
class FGSingleton {
static let sharedInstance = FGSingleton()
var gameScore: Int = 0
// METHODS
private init() {
println(__FUNCTION__)
}
func displayGameScore() {
println("\(__FUNCTION__) \(self.gameScore)")
}
func incrementGameScore(scoreInc: Int) {
self....
20
Very regularly when dealing with asynchronous stuff (among other things) in iOS development, we have to make a decision between three tools.
Closures (blocks, callbacks, etc)
NSNotificationCenter
The delegate pattern.
Which one is right for you depends on the scenario. It won't always be the same one. As a general rule, we can use the number of objects ...
19
Your code looks good to me, and I have only some minor remarks and
suggestions.
An Optional has an implicit initial value of nil, so
var retnValue: String? = nil
can be simplified to
var retnValue: String?
In
func generate() -> GeneratorOf<String> {
var value: Int = self.startValue
return GeneratorOf<String> {
return (...
18
So Apple already has an implementation for this, using NSNumberFormatter. A bit easier to implement than what you have now ;).
var numberFormatter:NSNumberFormatter = NSNumberFormatter()
numberFormatter.numberStyle = NSNumberFormatterStyle.SpellOutStyle
var string = numberFormatter.stringFromNumber(100)
18
First and foremost, printing text to the console is absolutely pointless for an iOS application. It's okay to do it as an easy way to test whether our code is working as intended, but we don't want to mistakenly leave this in the final release build.
So, step 1: Follow the instructions in this StackOverflow post to set up a custom compiler flag for your ...
17
I think this should work. The key (pun intended) to making this work is to take advantage of the tag property of UIButton. In Interface Builder, set a different tag to each piano key. Start with 0 for "C3", 1 for "C#", up to 12 for "C4". Wire all of those keys to playKey.
import UIKit
import AVFoundation
class ViewController: UIViewController {
...
16
The following is similar to Flambino's approach, but creates a Swift
SequenceType so that you can use the Swift library functions
filter() and reduce() to iterate over
the elements:
struct FibonacciSequence : SequenceType {
let upperBound : Int
func generate() -> GeneratorOf<Int> {
var current = 1
var next = 1
...
16
The first thing that stands out to me is that you've created an entirely new class that exists purely to have this class method. This doesn't make a whole lot of sense. At a minimum, this method could just exist on its own, outside of a class, but I'll do you one better: let's write it as an extension to UIDevice, that's where it makes the most sense, and ...
15
You can remove the size parameter as arrays know their own size in Swift, and use the zip, map and reduce functions to perform the computation rather than the C-style for loop, which is now unavailable in the latest versions of Swift (as is i++):
func mac(a: [Float], b: [Float]) -> Float {
return zip(a, b).map(*).reduce(0, +)
}
The zip function ...
14
assert(false, "Don't use assert like this!")
func input( x: [Double] ) -> Double
{
guard let inputLayer = layers.first else { return -2.0 }
guard let outputLayer = layers.last else { return -3.0 }
assert( inputLayer.neurons.count == x.count
&& outputLayer.neurons.count == 1 )
The logic for this method is all over the ...
13
Lately, when writing Code Review posts, I like to point out the first thing I notice when I look at someone code.
This is the first thing I notice when I look at your code.
Without looking at anything else, I can already tell you that your code is way too deeply nested and therefore absolutely confusing.
With looking at your code, I can tell you that not ...
13
So first, as CAD97 rightly points out, if you simply define your protocol as identically matching the methods from UserDefaults you intend to use, then making UserDefaults conform to the protocol is much simpler:
extension UserDefaults: UserDefaultsProtocol {}
And there ya go, done. Having to write less code is nice. What's even nicer here is that we're ...
12
In a for loop in Swift, the parentheses are optional, and from all of Apple's book and sample code, they are usually omitted--you did so yourself on the if statements. Furthermore, Swift has a range operator (two, in fact), so you should use that instead of the manual increments anyway.
for i in 1...100 {
...
}
For better or worse, == 0 is the only way ...
12
== false
while found == false
We'd prefer to see either this:
while !found
or this:
while notFound
(where in the second example, we reverse the boolean logic you've used)
Vertical White Space
A blank line between every line of code is not productive. It adds wear to my scroll wheel and makes your code harder to digest.
Instead, we should think of ...
12
I can see no advantage of computing the hash value from Int(self.x) and Int(self.y). As you already noticed, truncating the floating point
numbers to integers loses information and therefore causes
hash collisions.
CGFloat is (like all Swift number point types) Hashable, and its
hashValue is just the integer with the same memory representation
(as one can ...
12
Your function uses global variables, which is bad for several reasons:
The variables must be reset before the function can be called again.
The variables can be modified from outside of your function, causing
wrong results.
The function is not thread-safe.
In addition,
The program logic is not immediately obvious (at least it wasn't to me).
Calling the ...
11
I can only suggest a faster* math-based algorithm:
The fibonacci numbers are: 0, 1, 1, 2, 3, 5, 8, 13, 21, 34 and etc.
The even ones are: 0 at index 0, 2 at index 3, 8 at index 6, 34 at index 9 and so on.
This suggests that every third fibonacci number is even (starting from 0). Let's try and prove this by induction:
WARNING MATH AHEAD
We have already ...
11
func backgroundExecutable(f: (() -> Void) -> Void) -> (() -> Void)
This is madness and it takes a number of mental gymnastics to figure out what is going on here.
This method takes, as its only (and poorly named) argument, a closure which returns void and takes, as its single argument, a closure which returns void from taking no arguments. ...
10
The primary problem I see here is the way we're keeping track of the time passed. NSTimer is not guaranteed to tick with the exact amount of time you passed it. It will try getting as close as possible, but the most specific problem is when the thread has a lot of activity on it.
If between two ticks, the thread the timer is one gets a lot of activity ...
10
You are using a brute force attack to solve this problem but you should better think about it and use some maths.
Right now you are checking if the number is evenly dividable by 2, 3, 4, 5, 6 etc.
If you reverse the logic for instance you first check if it is evenly dividable by 20 you know that the number is also evenly dividable by 2, 4, 5 and 10.
...
10
One things for certain... an array of tuples which is basically being used as a glorified dictionary is never going to suffice as an acceptable data model. Moreover, an array of bytes isn't really that much useful than what we start with.
So what's clear, we need an actual data model. So names and data types may need some twiddling, but here's what I ...
10
Optimising the functional approach
Iterative vs functional approach: Is it possible to use the (existing) functional methods without losing performance?
I don't think it's possible to use the standard library's existing collection methods without losing performance here – as I go onto investigate, there are quite a few inefficiencies with them. We can ...
10
That is not the Sieve of Eratosthenes
The Sieve of Eratosthenes computes multiples
of each found prime to mark subsequent composite numbers in the sieve.
Your algorithm computes the remainder of all subsequent numbers instead.
That makes a huge difference. I'll come back to that later, let's start
with a
Review of your current code
There is a ...
9
You've posted quite a lot of code, and as such, for now I'm going to focus simply on a big picture overview of your code (and its organization).
This is the first thing that stands out to me is that you've got property declarations interspersed through all of your method declarations. Move all of the properties to the top.
There's a lot of code in ...
9
I would have started by looking over more than just the question in the linked question. You should have looked at the answers.
First of all, while the added where clause option in switch statements is a nice addition to the language, it doesn't make anything better here.
Moreover, we're failing on the single-responsibility-principle by not extracting out ...
9
let appDel: AppDelegate = UIApplication.sharedApplication().delegate as! AppDelegate
let context: NSManagedObjectContext = appDel.managedObjectContext!
The type annotations are not necessary, the Swift compiler can infer
the type automatically:
let appDel = UIApplication.sharedApplication().delegate as! AppDelegate
let context = appDel.managedObjectContext!...
9
readStrings() is better named readCharacters() because that is what it does.
I prefer Array(...) instead of .map { $0 } to convert a sequence into an array,
but that is a matter of taste:
/// Reads one line from standard input and returns the result
/// as an array of characters.
func readCharacters() -> [Character] {
guard let line = readLine() else ...
Only top voted, non community-wiki answers of a minimum length are eligible
