I have written my first iOS app in Swift, a Hacker News Client for iOS. I am a beginner iOS developer. I need some feedback from the Hacker News community about my code quality so that I can improve myself in the future. Be brutal and honest about what you think of my code structure and style.The project can be found at the following URL https://github.com/NikantVohra/HackerNewsClient-iOS.If you want to take a look at the app it can be downloaded from https://itunes.apple.com/us/app/hn-app/id983203003?ls=1&mt=8.
Have they fixed testing with Swift yet? I am normally a pretty test driven person, but honestly, Apple makes it pretty annoying to use tests with Swift, so I don't blame you for your lonely scaffold.<p>I'd say overall it looks pretty well written app code, especially if you say you're a beginner. One thing I'd like to point out is that in my (limited Swift) experience, stuff like this:<p><a href="https://github.com/NikantVohra/HackerNewsClient-iOS/blob/master/HackerNews/StoryCell.swift#L45-L49" rel="nofollow">https://github.com/NikantVohra/HackerNewsClient-iOS/blob/mas...</a><p>...where you're force casting/unboxing/whateveritscalled a bunch of things is a smell. Sometimes it's unavoidable because of the weirdness between Cocoa and Swift, so typically your interface builder outlets will be riddled with that stuff, but for your own internal APIs, try and keep things as option-less as possible.<p>UI code is pretty challenging to keep organized, especially when platforms encourage using things like two-way bindings and keeping state all over the place. I'd recommend checking out Gary Bernhardt's talk "Boundaries" and Andy Matuschak's similar writings (sorry, can't think of anything off the top of my head), as they have some good ideas for demarcating the line between functional parts of your code that should be super clean and the anything goes world of external UI APIs.<p>Good luck and keep coding!
Looks pretty nice and clean.
I don't know how you are using firebase in the app. But it seems you aren't using authentication.
Which basically means that anyone can delete all the data in your firebase instance.
I had a look at your code and at a first glance it looks quite good to be your first Swift project. As the others already said, you're missing tests that makes refactoring a bit hard for someone who's new to your code.<p>Other 2 points I noticed is:<p>1) You're using CocoaPods but you included SwiftyJSON, Alamofire and a couple of other libraries as plain source code. Why not submodules at least? This makes it hard to update the source of the external dependencies to the latest version<p>2) You're doing way too many casts in your code. This means that something could be wrong on an architectural level. I would like to help you (I already cloned your repo) but I'm using Swift 1.2 (btw your code is still on Swift 1.1) and I didn't manage to migrate because of point 1).<p>Apart from that, good job on shipping your app! :)
I'm not a ios developer, but you can find some other feedback on <a href="https://codereview.stackexchange.com/" rel="nofollow">https://codereview.stackexchange.com/</a>
Don't know much Swift - but I just wanted to say congrats on shipping something and also for the app - it looks really nice.<p>Good job.<p>Regarding the code quality - from my experience you'll be able to also see for yourself if the code is well written when you'll want to change something in the app(e.g. in the next update) and see how easy (or not) it is to do so.<p>Also my advice will be to take a look at other open source projects and try to see how others have done the same things - what was their approach and how is this different from what you would do in a similar scenario.
I've not written any swift apps yet, but I have written some objective c and the (effectively) empty test directory is definitely an indicator that this code will be hard to change.
For those advocating automated tests. Are you actually iOS developers with experience testing ios code? If so, I'm curious what frameworks/tools you've successfully used for unit and integration testing.
Congratulations on shipping your code!<p>I'm not a Swift or iOS developer really, so I don't know how those projects are supposed to be organized, but I glanced through your repo and I can see that you seem to be pretty good at breaking things up into small functions and stuff seems readable and well formatted enough to me. Good luck!
It's useful to get human feedback on one's code, but human attention
is scarce, and human judgement is influenced by many non-technical
factors (e.g. mood, politeness or hostility) that do not derive directly
from code quality.<p>For these reasons, I recommend complementing internet-based code
review by measurement:
Use (automated) testing in multiple forms, unit, integration,
randomised. Count the number of bugs you encounter. Classify the bugs
you find, track how the numbers and classes of bugs in your code
evolve over time. Compare those statistics with other coders. This not only
gives you ideas about your relative coding ability, but will also reveal areas where you could try and improve your abilites.
In my experience it's useful to add a little more documentation/comments to your code. It makes it a lot more readable for anyone else and more importantly, a lot easier for yourself when you're writing updates a while from now.<p>I like to have a little comment above every function that explains what it does at least. On top of that I usually put a single line above every logical block of code. Something like "Loop trough list of someobject." It seems obvious but it does make it a lot easier to read back code later.