Live data from Hacker News

When hiring developers, have the candidate read existing code

freakingrectangle.wordpress.com

201–210 of 565 posts

Re: When hiring developers, have the candidate read existing code

#201
I like this idea. My first job out of university, this was actually what they did.

Though many companies don’t care if you’re familiar with the specific language they use, because they assume a decent developer can pick up a new language, and if someone weren’t familiar with that specific language it maybe could put them at a disadvantage. Or maybe that would actually be a good test of their skills??

Re: When hiring developers, have the candidate read existing code

#202

Earlier quoted context omitted.

> Bonus points for no PR’s and trunk driven development as that shows a very mature team. Ugh, pass. Trunk development is fine. Skipping PRs just brings back nightmares of SVN. Even if 90% of PRs are approved without comment, it's extremely helpful for everyone to have a second set of eyes on work before it is merged in.

Skipping pr’s is not equal to skipping code review. If you pair, there’s two sets of eyes, to commit both pairs have to sign a commit. You can also organise a demo/quick mob session before commit. Then there’s a level of trust in your teammates. PR’s are great for open source projects as act as gatekeeper so not everyone can commit freely. If you need to gate keep your team members then I’d question the strength of y…

> If you need to gate keep your team members then I’d question the strength of your team.

I'm sorry, what?

Review is a gate for everyone, and is a sign of basic project maturity. WebKit, Mozilla, Chrome, LLVM, Linux, etc are all review gated projects. No change is landed - can be landed - without review. If you're questioning the strength of those teams I cannot imagine what your team would need to have on it??

Re: When hiring developers, have the candidate read existing code

#203
post #155

Earlier quoted context omitted.

When malloc returns NULL, it's saying that there was some error. However, IIRC the only malloc error is ENOMEM. It's unclear why malloc(0) would run into that. (If malloc(0) did run into an ENOMEM, then NULL would be required, but the result of malloc(0) need not tell you anything about subsequent calls to *alloc functions. However, there is a possible malloc guarantee to consider.) There's some interaction with real…

That's right ! Great answer. malloc() is a very old API. A more modern version would probably looks like: err_code malloc(size_t size, void **returned_ptr); The current malloc() overloads the return to say NULL == no memory available/internal malloc fail for some reason and as far as the standard goes, allowing NULL return if size==0. So if you get NULL back from malloc, did it really mean no memory/malloc fail, or z…

Except that I left out a "should".

While it would be good if malloc(0) always returned a pointer, you can't rely on malloc(0) returning NULL just for errors. There's not even a guarantee that malloc(0) does the same thing every time.

Note that "returning a pointer to the byte after the internal malloc header" means that malloc(0) == malloc(0), breaking the unique pointer guarantee unless malloc(0) actually causes an allocation.

However, allocating in malloc(0) means that while(1) malloc(0); can cause a segfault, which would be a surprising thing.

Re: When hiring developers, have the candidate read existing code

#204

Earlier quoted context omitted.

I think you missed what I said; there's no need to update the external doc just because you updated the value. The external doc is an explanation of what the value does , and what happens when that value is updated, not why it's set to a specific value.

That sounds wonderful in theory but it doesn’t seem realistic in general to have this doc explaining a particular constant and why/how it was chosen that doesn’t need updating if someone comes along and chooses a new value. I appreciate that you think you’ve discovered the answer here, but experience tells me it ain’t that simple.

Care to elaborate on why your experience tells you this?

I’ve put this into practice numerous times with positive effect, so my more relevant experience tells me this is a winning strategy, compared to in line docs, which are objectively worse in nearly every way.

Re: When hiring developers, have the candidate read existing code

#205

Earlier quoted context omitted.

> Bonus points for no PR’s and trunk driven development as that shows a very mature team. Ugh, pass. Trunk development is fine. Skipping PRs just brings back nightmares of SVN. Even if 90% of PRs are approved without comment, it's extremely helpful for everyone to have a second set of eyes on work before it is merged in.

Skipping pr’s is not equal to skipping code review. If you pair, there’s two sets of eyes, to commit both pairs have to sign a commit. You can also organise a demo/quick mob session before commit. Then there’s a level of trust in your teammates. PR’s are great for open source projects as act as gatekeeper so not everyone can commit freely. If you need to gate keep your team members then I’d question the strength of y…

The prs where dogmatic as more junior teams get caught up in superficial things such as names, package structure, syntax preferences rather than what the pr actually does.

All this means is that you and your team failed to learn anything from the PR process. If simple things like naming or syntax repeatedly come up then you have a style convention but the devs are ignoring it. The very obvious solution to that is to enforce the rules with a linter, and run the linter on a commit or push hook. If the linter fails then the dev can't open the PR until they fix the issues.

If you have process that's only for box checking and not something that's actually providing useful data the you're not using your time well enough. Removing the process is one solution, but it's not a very good one. Making the process useful is significantly better.

Re: When hiring developers, have the candidate read existing code

#206

I like this approach. Far to often I’ve interviewed at places and been grilled by the interviewer only to find out when you start the quality isn’t great, what you where grilled on you won’t be working on “as that’s to hard” or “we don’t do that” despite being grilled on it and the level of skill not to great they just want senior people. It’s the bait and switch. At least being taken through existing code you know w…

> Bonus points for no PR’s and trunk driven development as that shows a very mature team. Ugh, pass. Trunk development is fine. Skipping PRs just brings back nightmares of SVN. Even if 90% of PRs are approved without comment, it's extremely helpful for everyone to have a second set of eyes on work before it is merged in.

I agree, the PR is invaluable to preserve context: the trunk history remains simple and linear thanks to squash merges, and you can still see the fine grained commits (and matching discussions/reviews) in the closed PR.

It's also a way to give insight when onboarding new developers: if something is surprising, they can see why things came to be this way, not just accept the result at face value.

Re: When hiring developers, have the candidate read existing code

#207

I like this approach. Far to often I’ve interviewed at places and been grilled by the interviewer only to find out when you start the quality isn’t great, what you where grilled on you won’t be working on “as that’s to hard” or “we don’t do that” despite being grilled on it and the level of skill not to great they just want senior people. It’s the bait and switch. At least being taken through existing code you know w…

> Bonus points for no PR’s and trunk driven development as that shows a very mature team. Ugh, pass. Trunk development is fine. Skipping PRs just brings back nightmares of SVN. Even if 90% of PRs are approved without comment, it's extremely helpful for everyone to have a second set of eyes on work before it is merged in.

i think that was sarcastic.

Re: When hiring developers, have the candidate read existing code

#208

Please comment your code, when it is is necessary. I don't need "we need to loop here from 1 to 50", I need "we have to rate-limit this function to under 60 transactions per second due to hardware requirements", etc. If you are putting "magic numbers" anywhere, COMMENT it as to what that number is, why you chose it, etc. I'm 30 years into this game and I still come across code that takes way too long to reason about.

> If you are putting "magic numbers" anywhere, COMMENT it as to what that number is, why you chose it, etc. Better yet, turn magic numbers into constant variables whose name becomes the comment. Of course, comments can also provide additional context :)

Agree. I also add to this - name your constants by meaning, not value. Too many times I see

    const ONE_HOUR_IN_MS = 3600000
Instead I would like to see

    const RESEND_DELAY = 3600000

Re: When hiring developers, have the candidate read existing code

#209
post #166

Earlier quoted context omitted.

I'm sorry, no, there's plenty of code that requires much more than two sets of eyes, and outside discussion, for any project above a certain size. I trust your experience that the teams and projects worked out like that, but they must have been suitable to that approach, which is definitely not universal. As one example, do you think cross-functional changes to the Linux kernel from even trusted contributors can just…

Sure there are a 1% of megaprojects that require additional process, but for the rest PRs are a method to control code quality socially. They introduce delays and foster ego antagonisms, so less methodical ways to control quality are optimal if the requirements are met (buy-in + skill) and complexity isn't too great.

No, every project I have worked on has review gated commits. It is a /basic/ step in ensuring that a project maintains a high quality codebase. Review does cause delays, because reviewing takes time, but we've generally found that the speed "gained" through poor change control is more than made up for through bad code.

Review also shouldn't be causing ego antagonisms. You're coworkers. You have to be able to work together. That's called having a job.

Every project I have worked on in a professional setting has had mandatory code review, with review acting as a gate. That's been required in order to maintain software quality despite the quality of engineers I've worked with.

Re: When hiring developers, have the candidate read existing code

#210

Earlier quoted context omitted.

> Bonus points for no PR’s and trunk driven development as that shows a very mature team. Ugh, pass. Trunk development is fine. Skipping PRs just brings back nightmares of SVN. Even if 90% of PRs are approved without comment, it's extremely helpful for everyone to have a second set of eyes on work before it is merged in.

Skipping pr’s is not equal to skipping code review. If you pair, there’s two sets of eyes, to commit both pairs have to sign a commit. You can also organise a demo/quick mob session before commit. Then there’s a level of trust in your teammates. PR’s are great for open source projects as act as gatekeeper so not everyone can commit freely. If you need to gate keep your team members then I’d question the strength of y…

> superficial things such as names, package structure

These two are not superficial, though. Naming things is one of the Hardest Things, and that and structure tell you if you're in the right spot trying to track down a bug or add a feature.

Every debugging session should not be an adventure.

Post reply on HN