Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Ask HN: Is my code any good?

news.ycombinator.com

Post reply on HN
Print view View original

31–40 of 44 posts

Re: Ask HN: Is my code any good?

#31

Post by mediascreen »

cFire wrote: post #24
cFire wrote: post #22

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 tr…

To illustrate, here's something I wrote not too long ago: 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.

I like what Jeff Atwood wrote: "Code Tells You How, Comments Tell You Why"

http://blog.codinghorror.com/code-tells-you-how-comments-tel...

Re: Ask HN: Is my code any good?

#32

Post by kyberias »

cFire wrote: post #29
kyberias wrote: post #27

Earlier quoted context omitted.

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 y…

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.)

One good resource for learning and inspiration is Robert C. Martin's book Clean Code. Opinionated, yes, but a good read none the less.

Re: Ask HN: Is my code any good?

#33

Post by jacquesm »

cFire wrote: post #22

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 tr…

Comments should clarify, not repeat what the code already says.

Re: Ask HN: Is my code any good?

#34

Post by andrey-p »

kyberias wrote: post #27
cFire wrote: post #24

Earlier quoted context omitted.

To illustrate, here's something I wrote not too long ago: 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.

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 y…

Besides unit testing, with regexes I always put a quick comment like "matches foo but not bar or baz" which might not be comprehensive but allows the next person to get a quick understanding of what the code does at a glance.

Re: Ask HN: Is my code any good?

#35

Post by rimantas »

eric_h wrote: post #12

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.

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.

Re: Ask HN: Is my code any good?

#36

Post by enraged_camel »

mafribe wrote: post #10
nikant wrote: post #4

Earlier quoted context omitted.

I wanted to get some feedback on my code design and structure. I think that is difficult to get via automated code review.

That's not what your question asks for. Anyway, in my experience, the ability to test with ease is one dimenision of design quality.

>>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."

Re: Ask HN: Is my code any good?

#37

Post by rimantas »

ninjakeyboard wrote: post #25
nikant wrote: post #16

Earlier quoted context omitted.

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?

It's not simply best practice - it's necessary to maintain your code.

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.

Re: Ask HN: Is my code any good?

#38

Post by amorphid »

nikant wrote: post #16
GaiusCoffee wrote: post #14

After several years in a test-driven development team, seeing your test directory made me cringe :/

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?

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.

Re: Ask HN: Is my code any good?

#39

Post by yoklov »

kyberias wrote: post #27
cFire wrote: post #24

Earlier quoted context omitted.

To illustrate, here's something I wrote not too long ago: 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.

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 y…

Worth keeping in mind that you don't always want to split something into a separate function. When you split out a function, you should be asking yourself if you're actually simplifying something, or if you're just sweeping the complexity under the rug. If the person reading the first function will typically need to see the definition of the helper, the helper's definition is non-obvious, and the helper wouldn't be used elsewhere, it's probably not worth splitting out.

Another important consideration is whether or not the code block you split out makes any assumptions about the current state of the program (or object or whatever), and if those assumptions not being met would result in non-obvious bugs. (If this is true, you should probably wait until there's actual duplication somewhere before splitting the function out, IMO.) This is obviously not an issue with pure functions, but for most programs, most functions aren't pure.

See http://number-none.com/blow/john_carmack_on_inlined_code.htm... for further perspective on this.

Re: Ask HN: Is my code any good?

#40

Post by richardwhiuk »

cFire wrote: post #30
xavierd wrote: post #28

Earlier quoted context omitted.

I would say you're documenting way too much: the obvious shouldn't need documentation.

It's more a matter of what you consider to be obvious in this case. In my experience, especially regex matching can become very not-so-obvious if you're coming back to code you wrote some time ago. Or if someone else is reading it.

		# Do NAT table lookup
		n = natlookup( p1, p2 )
is clearly unnecessary. If natlookup doesn't do a NAT table lookup, it's a badly named function.
Post reply on HN