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

How I review code

engineering.tumblr.com

Post reply on HN
Print view View original

111–120 of 144 posts

Re: How I review code

#111

Post by watwut »

lukevers wrote: post #41
watwut wrote: post #35

Earlier quoted context omitted.

> Once the less than ideal code is in the codebase it's there to stay. That is not objective fact across projects. That means either the process or culture is bad.

Not exactly. In a lot of fast paced environments it’s rare to get a chance to rewrite something that functions properly just because it could be labeled as technical debt. It’s not about culture, it’s about moving on to the next thing. I code review hard to make sure we have the smallest amount of technical debt possible because it’s usually there to stay for a considerable amount of time.

That would fall under "culture".

Re: How I review code

#112

Post by 0xCMP »

pencilhappen wrote: post #87
athenot wrote: post #70

Earlier quoted context omitted.

Why? Having many languages allows to pick the sweetspot for each subsystem. Each language has special strengths and weaknesses, there is no silver bullet that excels at everything. Go, C/C++, Lua, Ruby, Perl, Scala, Node.js, Python... each of these are THE best choice for certain classes of problems (and terrible for others). It may be because of language features that elegantly express a solution, or particular effi…

> Each language has special strengths and weaknesses, there is no silver bullet that excels at everything. Go, C/C++, Lua, Ruby, Perl, Scala, Node.js, Python... each of these are THE best choice for certain classes of problems (and terrible for others). Not quite true... not all languages have a sweet spot in a production environment. Node.js isn't particularly excellent at anything, the attraction is mainly "I know…

Ruby, Python, Perl, PHP, and (maybe) Lua aren't amazing at handling async operations like Node.JS can.

Go is, in many respects, a great replacement for Python and Node in that it's simple and handles concurrency well. Scala and other JVM languages fall here too, but come with their assoc complexity.

Python, Node, and Ruby have tons of battle-tested code which often makes the language the "strongest" if for nothing else than there is code that works and it's not critical enough we rewrite it ourselves.

There are definitely sweet spots for each of these languages.

Re: How I review code

#113

Post by efficax »

pmcollins wrote: post #106
athenot wrote: post #70

Earlier quoted context omitted.

Why? Having many languages allows to pick the sweetspot for each subsystem. Each language has special strengths and weaknesses, there is no silver bullet that excels at everything. Go, C/C++, Lua, Ruby, Perl, Scala, Node.js, Python... each of these are THE best choice for certain classes of problems (and terrible for others). It may be because of language features that elegantly express a solution, or particular effi…

But there are so many benefits to standardizing on one language (to take the extreme case). You create the business rules, APIs, and utilities in whatever language you choose and that can be shared by everyone. That becomes the core of your business. If you need something new, you create it and submit it for review, and then everyone can use it, not just the folks who choose what was your preferred language at the ti…

> And BTW, I have yet to encounter a problem that is hard to solve in my preferred language, but is way easier to solve in another. I think the key to that is to choose carefully what you're going to commit to.

Long-running light-weight servers are nearly impossible to write in PHP, but a breeze in Go. There are tons of problem spaces where the only available libraries are in C/C++ or Java, which means its going to be easier to solve the problem in those languages than it is to build a new library from scratch. Python has some of the best packages for a number of problems, like machine learning, data science, image manipulation. Even if Go is purpose built for web crawling, beautiful soup (Python) is still what I'd choose for getting a scraper up and running quickly. And of course since The Web Is King everybody has to use JS for something these days.

If you're a large shop doing a lot of things, it's going to be more work to stick to one or two languages than it is to pick the best language for the job.

Re: How I review code

#114

Post by jay-anderson »

anameaname wrote: post #3

A conundrum for me is how to get other people to code review the way I want to be code reviewed? Particularly, I noticed code reviewers on my team are pretty pedantic, obsessed with correctness, and need to be explained why each change is okay. These are people that regularly write good quality code themselves, but there is a high amount of distrust. Why doesn't a team of talented programmers trust each other? (in ca…

One thing we did to get past the most picky review comments was institute some static analysis with some agreed upon rules which are run automatically during build checks of pull requests (sonar and checkstyle). If someone brings up an issue along these lines that you believe is unrelated to the task at hand you can ask them to propose a change to the rules instead.

That said, in the end, correctness is important and justifying and communicating your change to others is the purpose of a change request. If you aren't following define standards and idioms then you should get on board. If they aren't defined then they should be defined and reviewed to get everyone on the same page. If everyone is using code reviews to grandstand or show off their knowledge that's a cultural problem that needs to be addressed.

Re: How I review code

#115

Post by dboreham »

cobbzilla wrote: post #72
dboreham wrote: post #52

Earlier quoted context omitted.

Well there's a can of worms! Thoughts based on experience: Asymmetric information situation: newly hired smart engineer says that new subsystem should be written in . He says it is so much better than . Everyone on HN says it is so much better. You however, haven't had the time to try it out on a medium sized project to determine if this is true or the usual new language hype. New Engineer seems to know plenty about…

The answer to both situations is "No" and "No". Many engineering managers have a difficult time saying "no", to the detriment of the business. We are building a product for customers, not a playground or post-graduate program. There are legitimate reasons to add another language but they must be evaluated with the needs of the business in mind, these include long-term maintenance costs and hiring/training costs regar…

This sounds great in theory, and in companies that I personally own I am able to achieve this level of control (mostly). However in my experience on the battlefield you rarely have the luxury of this level of absolute control.

Re: How I review code

#116

Post by didibus »

cobbzilla wrote: post #72
dboreham wrote: post #52

Earlier quoted context omitted.

Well there's a can of worms! Thoughts based on experience: Asymmetric information situation: newly hired smart engineer says that new subsystem should be written in . He says it is so much better than . Everyone on HN says it is so much better. You however, haven't had the time to try it out on a medium sized project to determine if this is true or the usual new language hype. New Engineer seems to know plenty about…

The answer to both situations is "No" and "No". Many engineering managers have a difficult time saying "no", to the detriment of the business. We are building a product for customers, not a playground or post-graduate program. There are legitimate reasons to add another language but they must be evaluated with the needs of the business in mind, these include long-term maintenance costs and hiring/training costs regar…

Talent retention is needed tp be considered also. Engineers do want opportinitues to learn new things and experiment. If you're too draconian you might see top engineers leaving or moral being low with discontent engineers who might in turn be less productive.

Re: How I review code

#117

Post by bluesnowmonkey »

dboreham wrote: post #52
pmcollins wrote: post #44

> We have repositories for the PHP backend, our database schemas, our iOS (Swift/Obj-C) and Android (Java/Kotlin) mobile apps, infrastructure projects written in Go, C/C++, Lua, Ruby, Perl, and many other projects written in Scala, Node.js, Python, and more Why do organizations allow this? I realize that some platforms require their own languages (iOS, Android), but outside of that, just pick one or two and hold the…

Well there's a can of worms! Thoughts based on experience: Asymmetric information situation: newly hired smart engineer says that new subsystem should be written in . He says it is so much better than . Everyone on HN says it is so much better. You however, haven't had the time to try it out on a medium sized project to determine if this is true or the usual new language hype. New Engineer seems to know plenty about…

Or you acquire a company that had a different set of approved languages. Do you throw their tech away and rebuild from scratch? No. N+M languages.

Re: How I review code

#118

Post by tome »

chriswarbo wrote: post #81

> I look for code that is well-documented (both inline and externally), and code that is clear rather than clever. I’d rather read ten lines of verbose-but-understandable code than someone’s ninja-tastic one-liner that involves four nested ternaries. "Clear" and "clever" aren't in opposition, and likewise "verbose" and "understandable" aren't correlated. I think this characterisation, and especially the example, show…

An HN gem of a comment!

Re: How I review code

#119

Post by cobbzilla »

news_to_me wrote: post #110
cobbzilla wrote: post #86

Earlier quoted context omitted.

If I read "good companies" to mean those that have enough excess resources to make lots of foolish mistakes, then yes, totally agree. "Keeping devs happy" by itself is a terrible reason to introduce a new language. If your developers cannot find happiness by working together and building something great, you have bigger problems.

> If your developers cannot find happiness by working together and building something great Just a friendly reminder that there are lots of different kinds of people who approach their work differently. Not being "business-driven" isn't necessarily a bad thing.

sure but wouldn't you agree that being in business at all, and not unemployed, is a bigger thing, and that the luxury to play is earned by success. if you can't pay people including yourself, I suppose you can always stay at home and play/experiment all you want.

Re: How I review code

#120

Post by cobbzilla »

didibus wrote: post #116
cobbzilla wrote: post #72

Earlier quoted context omitted.

The answer to both situations is "No" and "No". Many engineering managers have a difficult time saying "no", to the detriment of the business. We are building a product for customers, not a playground or post-graduate program. There are legitimate reasons to add another language but they must be evaluated with the needs of the business in mind, these include long-term maintenance costs and hiring/training costs regar…

Talent retention is needed tp be considered also. Engineers do want opportinitues to learn new things and experiment. If you're too draconian you might see top engineers leaving or moral being low with discontent engineers who might in turn be less productive.

There are always interesting problems to solve and new technologies and solutions to use. That fun definitely never ends. But a whole new language (not counting small glue/config languages/json dialects/etc), a new source base for the team, one must be cautious (and they always start small, more temptingly). It's a potentially huge and maybe not great investment being pitched. Do your due diligence.
Post reply on HN