7 ms·

Ask HN: Is my code any good?

• mafribe 11y ago
  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. 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.
  • nikant 11y ago
    I wanted to get some feedback on my code design and structure. I think that is difficult to get via automated code review.
    • mafribe 11y ago
      That's not what your question asks for. Anyway, in my experience, the ability to test with ease is one dimenision of design quality.
      • RussianCow 11y ago
        How is that not what was asked? The phrasing of the question makes it immediately obvious, at least to me, that the OP was looking for a design critique. (Unless of course the question was edited since you wrote your initial comment.)
      • enraged_camel 11y ago
        >>That's not what your question asks for. Maybe you didn't read it, but the question clearly and explicitly says: "Be brutal and honest about what you think of my code structure and style."
• WorldWideWayne 11y ago
  Congratulations on shipping your code! 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!
  • nikant 11y ago
    Thanks for looking into the repo. I appreciate your feedback :)
• gfosco 11y ago
  Looks pretty good to me. Congrats on launching an app!
  • nikant 11y ago
    Thanks :)
• alc90 11y ago
  Don't know much Swift - but I just wanted to say congrats on shipping something and also for the app - it looks really nice. Good job. 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. 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.
  • nikant 11y ago
    Yeah I am already looking into the next update and faced some challenges. Thanks for the advice. Will take care of the same in the future :)
• fmax30 11y ago
  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.
• eric_h 11y ago
  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.
  • rimantas 11y ago
    Empty test directory says absolutely nothing how hard this code will be to change. And seeing that this code is quite well organized, to change it won't be hard at all.
    • eric_h 11y ago
      The code is well organized, and as there's relatively little of it, it will be relatively easy for the author to change the code. As he starts to add features and accept pull requests, he will have to manually check via static analysis and manual testing to ensure that the code will still work perfectly. As the code base grows, doing it this way reliably will become more difficult, to the point that he will begin to fear change. There is simply no debate that a well tested code base is easier to confidently add features to, confidently refactor and debug than code with no tests at all. This is particularly true if you bring on new contributors who have had no experience with the codebase. Humans are not computers, they are never as good at repeatedly executing all possible execution paths of an application without growing tired or over confident that it doesn't need to check some path because it's sure it works. I would take a well tested codebase over a well organized one with no tests any day of the week, because I know I could confidently reorganize the code without breaking anything. (well tested does imply that the tests themselves are well organized, though). The OP was asking for criticism. He should add tests, lots of 'em.
• tigroferoce 11y ago
  I'm not a ios developer, but you can find some other feedback on https://codereview.stackexchange.com/ https://codereview.stackexchange.com/
• GaiusCoffee 11y ago
  After several years in a test-driven development team, seeing your test directory made me cringe :/
  • nikant 11y ago
    Yeah I know I need follow the best practices. I have started writing tests for the next update. But do you recommend TDD for iOS development?
    • melloclello 11y ago
      Devil's advocate: I feel like TDD for iOS is a bit of a wasteland, and don't usually bother with it personally, unless there's some especially dense mission-critical business logic that I need to be 100% confident in.
    • ninjakeyboard 11y ago
      It's not simply best practice - it's necessary to maintain your code.
      • rimantas 11y ago
        It is not. There are heaps of code with test which is unmaintainable and heaps of elegant code without test in sight which is easy to maintain. Cargo culting is rarely the best practice.
    • amorphid 11y ago
      If you wanna get a lot better, try writing your own testing framework. I wrote a BDD style testing library for Androod when I first started learning Java, and that helped me understand how to write good code for Android platform.
• zoskia 11y ago
  How long have you been coding total? Was Design+Code your sole source for learning how to program?
  • nikant 11y ago
    No I have been coding now for close to an year now. But this is my first project with Swift. I have also watched the video lectures of the iOS development course offered by Stanford on iTunes.
    • avinassh 11y ago
      The design (icons, graphics, screenshots) all look really nice. if you did all of that by yourself, then kudos!
• bigbento 11y ago
  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. 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: https://github.com/NikantVohra/HackerNewsClient-iOS/blob/master/HackerNews/StoryCell.swift#L45-L49 https://github.com/NikantVohra/HackerNewsClient-iOS/blob/mas... ...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. 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. Good luck and keep coding!
  • nikant 11y ago
    Thanks for the feedback :)
• cFire 11y ago
  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. 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.
  • cFire 11y ago
    To illustrate, here's something I wrote not too long ago: https://gitlab.insomnia247.nl/coolfire/pfsense-ident-proxy/blob/master/identproxy.rb https://gitlab.insomnia247.nl/coolfire/pfsense-ident-proxy/b... It's a little quick-and dirty but it's the way it's commented that makes the point I'm going for here.
    • kyberias 11y ago
      Here's an example. You write: # Check if it's something that looks like an ident response if( input =~ /^(\d+)(|\s),(|\s)(\d+)$/ ) p1 = $1.to_i p2 = $4.to_i else sane = false end Don't do this (comment a block of code). Rather make a function that does the same and name it for example "ValidateIdentResponse". Write a test for it. Call that function from this function. You get shorter, less-complex main function and you don't need that silly comment. (EDIT: Formatting)
      • cFire 11y ago
        Valid point. In this case a separate function would be better. It's not always an option though. Edit: Rather I should say; It's not always an option that makes things better. If you're writing a comment for a block of code is definitely a good time to think "Should this not be a separate function?" (I clearly need to do this more myself too.)
        • kyberias 11y ago
• vittoriom 11y ago
  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. Other 2 points I noticed is: 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 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). Apart from that, good job on shipping your app! :)
• tylerc230 11y ago
  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.
  • purans 11y ago
    We use https://github.com/kif-framework/KIF https://github.com/kif-framework/KIF for automation! But you know the deal on how comprehensively you can do automation test!