Unfortunately, not everyone has time or desire to work on personal projects. Being an engineer outside of work should not be a pre-requisite to being one at work.
as a more concrete example, as I get older I'm less interested in working on the computer off hours. I've noticed I don't even play video games as much as I used to. I actually started doing wood working a couple of years ago, and playing with 3d printers. It does go in fits and starts though - sometimes I get an itch to play with something just for fun...
I think swayvil's original point was that you could still speak of work you've done in the past. They said 'show' and 'personal', but I wouldn't take that so literally if your favorite project wasn't personal. Even if under NDA, and I'm a security consultant so I know all about those, usually it's still possible to discuss the general problem being solved and how you solved it, and have a good conversation about that.
That said, I do find this hard in interviews (when asked about past experience) and prefer to just be given a web application riddled with every type of bug and try to bingo them all in the time given to show that I understand them all. That one was my favorite interview. But that might just be a different style of interview for a different type of job.
We did this in the first interview and they did well on that.
Then we gave an exercise* to write some code. In my opinion a junior (but not noob) programmer should be able to do it: the code should loop through a file that is too large to keep in memory and collect some statistics -- e.g. look up numbers associated with keywords and compute the average (where the keyword list is the large file; the lookup table is like ten entries).
This candidate (like the last one) instead worked around the memory constraint and just split up the file using external software such that they could fit the whole thing in memory. Not combining the partial results later or anything, no, just compute answers based on this one slice. Then in the interview afterwards, they could not describe how this could have been done differently, also not with hints (I understand they're under pressure during the interview). The code also had other problems (e.g. opening and re-reading this lookup file inline in the main loop... which runs a billion times). Even though functional (if not always giving the correct result; mistakes happen), they had neither a concept of memory complexity nor computational complexity.
So that's just anecdotal, maybe it was an outlier. From my perspective, talking about projects and code isn't always the same as them being able to work on the problems you're hiring them for.
* Note this was not entirely my choice, even though with hindsight it was a good idea we should at least have compensated their time regardless of the outcome imo.
The trouble with this argument is that it excludes a huge proportion of the best developers out there. A lot of people got into programming because they enjoyed it but after doing it all day at work they want to do something else with their personal time. And the more experienced someone is with programming the older they are likely to be and the more likely it is that they will have other interests and commitments competing for that time.
Most contributions to opensource happen on company time atleast in my experience. I don't code much outside of work yet have plenty of OSS contributions because in the course of my work I discover bugs, desire new features, etc. All the best developers in these circumstances will atleast attempt to get their changes merged so they don't have to carry patches.
I'm not looking to interview, but if I would you'd find that my personal projects fall in categories of either being:
a) outdated, because almost all work relevant to my subfield have been at my job for a some years now (and my experience has grown accordingly, since), or,
b) irrelevant, because by now my personal projects are mostly orthogonal to my job (e.g. stuff involving electrical engineering and digital signal processing), which I am absolutely not applying for.
That's interesting, I've never heard testing code skills by reading instead of writing.
An example would have been nice though, as I'm not sure how to find a piece of code that does something standalone that is too large to grasp in 20 minutes yet make a reasonable prediction at the output. That combination seems kind of weird.
I wonder how well it would work to modify OP's idea and present a candidate with some code that has a bug in it (you reveal what the bug is to save time, they'll need to understand the code anyhow) and you ask them how they'd fix it. First broadly, like what approach, then actually write part of it.
To prepare for the interview, one could add an argument like -h to show usage (in the same style as -v is currently implemented), then tell the candidate that the bug is that passing -vh executes neither -v nor -h (one would commonly expect it to execute both). Fixing this requires a 'structural' change to this little bit of code, but it's small enough to easily grasp and implement. The candidate might propose to loop over each character after a single hyphen to fix this, or they might propose to throw out this custom reinventing-the-wheel and use a proper arg parsing library. Either is fine, but they should be prepared to write the code (in any language, they can write just the changes in C, Python, JS, or whatever language they're comfortable with).
Has anyone tried such an interview coding question?
Good idea. I'm not sure I know arg parsing libraries in every language though. Even so, hopefully talking about where to make the change and finding some good documentation on an arg parsing library would be good enough start for 20 minutes.
> I'm not sure I know arg parsing libraries in every language though.
In my opinion, you should be free to look that up: that's what you'd do on the job as well. I also google it every time I need to use arg parsing in literally any language because I need it fairly infrequently, and frankly that's how this code came to be: too lazy to look it up again, too easy to write myself, and enjoying the writing of code. Any other solution would also be fine, this test would be about seeing if you can read and write code rather than if you come up with the (what the interviewers think) is the perfect solution.
Yeah, we sometimes say stuff like "we don't expect you to know the arguments to sigaction just like that, you can ask us anything[1] or just put what you think could be reasonable", to make clear we're not looking to hire an encyclopedia.
[1] And if they do, we probably end up pulling up the man page for them because we don't remember either. It's just not the relevant part of any coding interview, the reading or the writing kind.
My favorite front end interview did this exact thing. The interview started with an issue taken straight from the Preact GitHub codebase. The interviewer provided the issue text and the commit right before it was fixed to pull down onto my machine. I had an hour to figure out how to build and reproduce the issue, Take in the high level structure of the code base, figure out how to drill down in a debugger to find where the issue itself was occurring, then propose a fix for it. Took the whole hour but was both extremely satisfying for me, and I imagine very insightful for the interviewer.
We used to have an interview process that had a take-home project.
The project was:
- you have two files: listings.json and products.json
- listings.json lists ~20,000 Amazon listings for electronics with fields like title, brand and price
- products.json lists ~1,000 digital cameras, with fields like brand, family, model
- your job is to write a script that, for each entry in listings.json, emits the best match (or no match!) from products.json
Your solution could be as naive or as fancy as you wanted. The main point was to have something for the next stage.
We'd run your submission, and use that to show you some false positives and false negatives. Then we'd ask you to debug why each false positive or false negative happen, explain it to us, propose how you'd fix it, and identify any trade-offs in your proposed fix.
Eventually, we wanted to offer a non-take-home-project interview. We already had a bunch of existing solutions from employees, so we used those to run a stripped down version of the interview that just focused on the code reading/debugging/proposing fixes part.
I think both of these interview approaches were pretty effective at giving candidates a natural environment to demonstrate skills that they'd actually use on the job -- debugging, collaboration, predicting downstream implications of changes, etc.
That's a great example that fits almost 1:1 with something we've been doing. I will propose that to avoid having to give take-home projects, thanks for sharing!
My company has an initiative to bring in students for a 3 day event where they attend workshops and talks.
We were brainstorming what type of content would make fun and educational workshops for students. I proposed something along OPs lines. I called them "debugging minichallenges". The idea is to present students with a buggy implementation for a simple problem, and they need to find and fix the bugs. Just as OPs argues, the concept is that reading and understanding code is a fundamental skill.
I have the set of sample implementations in my Github, for example the one in Python [1]. This is our first time running this workshop so I can report how well it did after the event.
This article would be much more interesting if they shared the code. They imply they use a new set of question every cycle so it should be safe and easy to share the examples used in a past cycle.
Those are similar to my own thoughts. I like also having them talk about their favorite recent achievements.
> I can quickly train a person to have knowledge in some domain,
This part I have a little bit of trouble with. If you have a trivial domain, sure. In my experience domain knowledge takes a while to truly get your head wrapped around. But I expect that's why there's always other things that people do in an interview.
Makes sense to me in general. We all spend much more time reading our own code and others', much more than writing code. In fact, first period a new hire is going to experience is spending a disproportionate amount of time, just getting to know the system(s) they're expected to contribute to.
I'm currently looking for someone to join our own team, I'll be taking this adjusted priority in mind.
This is exactly what I started doing at my company a few years back. You are presented with a complete app written in the stack we use. This app has some bugs we will solve to get it working (nothing that's a trick... actual, commonly encountered bugs that have all the error messaging you need to solve them). Once it is working you will walk me through a particular flow of the app, explaining what is going on and why we do it this way. There are optimization opportunities (purely optional, there for you to notice), areas we can dig into if we choose (or if I feel the need to). If you tear through it we can examine a different flow in the app. Perhaps we could discuss the database structure and how it might be improved or changed, maybe we dig into some CSS or into some GraphQL or any other aspect that we need to. I've had people stumped and unable to continue but the vast majority of devs can make a good stab at it, even if they are not familiar with the specific stack. The best devs are barely slowed down by such unfamiliarity and can still reason, logically, about the code.
So far I've had really positive feedback from job candidates. A couple even described it as their favorite interview ever! I feel like it has worked well, given the people we have ended up hiring.
Are you able or willing to share some samples? I've been shying away from doing technical assessments the "old way," instead focusing on having a discussion about a candidates past projects. I feel like your way could be a good chance to get insights into someone's problem solving and critical thinking skills, which is truly what we value as a dev team.
I can’t share the code, unfortunately. It’s a full stack CRUD app with some third party API integrations.
You’ve got it! Gaining insights into their problem solving and critical thinking is a primary goal (more important than fluency in the various frameworks used).
I only once ever had an interview as you describe, and from a candidate perspective, it was my favorite! I had fun with it, and it was far more collaborative with my interviewer. I got to learn from them, instead of just regurgitating straight CS knowledge. It was all around a more human centered process. I got to learn that I would enjoy working with my interviewer, for example (this was a smaller company, so probably less applicable at scale-- unless you can continue to have the interviewers be candidate's colleagues, at scale).
"Bugs we will solve" seems to imply there's still a coding component. Is that correct? That's a different thing from what OP describes (and FWIW I'm pretty sure I like yours better).
Yes, that’s fair. However the bug is really trivial to solve and does not require much beyond reading the console error and then making a tiny change to the server code.
Does "finding the source of a bug" and "figuring out the correct thing to write to fix a bug" fall under reading the code or under writing the code?
I feel like the answer to those questions determines which is faster. One may say that you only compare (lines_read / time_reading) and (lines_written / time_writing) to find an answer, but if we remove all "time spent thinking" from both then it kind of feels like a meaningless comparison.
I’m spend much more time thinking about what I will write than I do thinking about what I’m reading. In any case my reading is far faster than my writing.
I sometimes ask candidates "what kind of interview do you feel would best bring out your strengths?" and try to adapt the interview to their response if I can. It's helpful if they want to talk about side projects or war stories, but doesn't pressure them to. I still give my coding challenge after. Wonder why no one else does this.
On the contrary: I like the idea, but for context, my most recent HN comment contained:
> [...] That said, I do find [it] hard in interviews [to talk] about past experience[s] and prefer to just be given a web application riddled with every type of bug and try to bingo them all in the time given to show that I understand them all. That one was my favorite interview.
Now I see johnqian suggesting to ask interviewees how to best see their skills, and it sounds great. If the interviewer doesn't have something like this on hand, we could still go into different types of bugs that they care about.
I've also had interviews that went terribly, particularly when no technical details were asked at all for a technical position (e.g. a manager just asking the standard questions where you need to give platitude answers, such as "why are you specifically excited to intern with Vodafone", methinks: "because I want that degree and you were the only option I could think of within 45 minutes commuting"... but one can't say that so I made something up.. he probed further on that... it went downhill from there), so being able to steer away from that sounds great.
But it might also be that I would feel unprepared for such a question and give a total non-answer, helping neither party.
Asking this in an email ahead of time might be better, though then the effect you're suggesting might be even stronger.
Edit: I'm not the downvoter, to be clear. I think you raise a good point even if I am not sure whether it would necessarily be that way.
The standard coding challenge is still most of the interview, so that provides some consistency. I just want to give candidates an additional opportunity to flex, in case they feel there's an important thing that most interviews neglect. Most candidates never really thought about it, and that's totally fine, for them I just do the standard course.
As for avoiding unconscious bias, was that ever an option?
Ha! One could think that the psychological screening is meaningless... until you are involved in hiring people. Of course, 95% candidates will pass, otherwise humanity would be doomed
> What are your strengths and weaknesses,.. hey what is this a psychological scan?
That's exactly what it is, and it's important. It's not helpful over the long run if we hire someone who can crank out good code, but their mindset negatively infects the rest of the team, causing morale to drop and people to leave.
I give technical interviews, but I'm also evaluating soft skills while I do it. I would rather have a team with ok-to-good technical skills, than a team with a rockstar prima donna who sabotages the cohesiveness of the whole.
Very little empirical evidence supports it, despite people parroting how "useful" it is. You're also teaching people to BS about themselves to get a job, perverting the entire thing.
If you want to psychologically analyze people, use a method which is actually supported empirically instead of the "do what every other pseudo-psychologist does" method. Big 5, for one, actually has some empirical evidence supporting it, but is barely ever used.
On the other hand, if you can actually figure people out in an hour under a single set of constraints, somehow being able to extrapolate that to the work environment as a whole, while also trying to put the work environment in a much better light than it actually is: quit your IT job now and make millions selling books.
> It's helpful if they want to talk about side projects or war stories, but doesn't pressure them to.
I like leading with that, and am a big fan of the initial "what's an interesting problem you had to solve?" question, but I made the unfortunate experience that even people with good war stories are sometimes very shaky in the basics of the particular subfield I'm interviewing for. To the point of deeming someone not a good match for our team, but encouraging them and internal recruiters to interview with other teams.
There is ultimately no full substitute for getting down to brass tacks and actually do some "sample work" together. Whiteboard coding is the usual (and, I guess, often poorly implemented) approach, but I've used reading existing code, with added intentional mistakes, to great success so far.
EDIT: Just read your new reply in a sibling thread that you're doing that anyway, and project and war stories are supplemental. It seems like we're in agreement, then!
When hiring at scale we value consistency so this wouldn't work well. However in my company we started an initiative called "candidate experience". In one of the interviews we allocate some time for the candidate to "flex" as you put it. We ask open ended questions such as "what is your proudest achievement?" or "is there anything you wanted to tell us but didn't had a chance to?". These questions are not used to evaluate the candidate and often are no even recorded, and gives a chance for the candidate to relax and put forward their best self.
> these questions are not used to evaluate the candidate
> gives a chance for the candidate to ... put forward their best self.
How does that work? If it genuinely won't be evaluated, what's the point of getting their best self? How do you make sure things you learn in this don't affect your judgement later?
This is my standard practice - I give a real world data fix script that has been simplified to understand and comment on in 20 minutes. The interviewee is the code reviewer before it gets run in production. It covers things like performance, security, typical syntax mistakes, and working with sensitive data. The best part is actually not the script, but the stories it triggers about past challenges.
> The interviewee is the code reviewer before it gets run in production.
Yeah I've also been using this approach for Fullstack Devd: A small page with a bit of CRUD + a small ticket description of what the page is supposed to do.
The code contains various bugs or questionable implementations, the interviewee is supposed to analyze the code and to either fix the issues right away or to write comments.
Nobody is expected to get everything right within the time slot, but I've found it to be a great test on how a candidate might perform in their day to day work.
My one big tip: One vice that I see among hiring managers is an unwillingness to ask tough follow-up questions. If you ask a question and there is any vagueness in the answer, you need to drill down deep until all vagueness is eliminated, so you understand exactly what the person knows. Follow up on what's said, but also follow up on what is not said.
Here’s a real-life example. I asked a recent applicant (for a fullstack software job, where we were hoping to hire a novice-to-mid-level engineer):
Me: How would you improve a situation where a page is loading slowly and you suspect the problem is related to the database?
Applicant: Well, I’d start by checking the HTML, is it correctly done, and then the CSS, is there any redundancy? And then the Javascript, is it correctly written, is it minified? Can we speed that up at all? Check the timeline, the API calls, see what is slow.
Me: Okay, great, that’s a good start, but what else? If the problem is not on the frontend, then what?
Applicant: Uh, well, then, I guess I need to look at the backend database model code. Is my database model code concise? Am I fetching the data needed, without any excess?
Me: Okay, great, that’s a good start, but what else?
Applicant: Uh, what else? Well, uh, we really need to look at that database code. Is the model bloated? Can we slim it down?
Me: Yes, okay, you basically said that already, anything else?
Applicant: Uh, well … uh, you need to check the HTML and the CSS and the Javascript and then, uh … API calls … uh ... the model code, make sure that is cleaned up. That needs to be lean.
Me: Yes, okay, but you said all of that already, anything else?
Applicant: Uh … well … the model code … and uh …
Me: Have you ever worked directly with a database?
Applicant: Uh … not much?
Me: If you get unexpected results from your model code, do you know how to debug the query?
Applicant: Uh … I guess I could … not really.
Me: Have you ever looked at the “slow query” log?
Applicant: Uh … no?
Me: Do you know how to run EXPLAIN or ANALYSIS?
Applicant: Well … uh …. no.
Me: Have you ever written SQL by hand?
Applicant: Uh … no.
Me: Are you aware of any differences in dialect between the SQL of MySQL and the SQL of PostGres?
Applicant: Uh … no.
Basically, they were somewhere between a novice level and a mid-level engineer, so they knew the frontend reasonably well, but they didn’t know a thing about databases. Which was okay, because that was what we were looking for. We still hired them and they turned out to be great in some areas, and they were eager to learn about the things they didn't already know. But obviously, if I'd been hiring a senior-level engineer, and it turned out they knew nothing about databases, that would have been a problem. The crucial thing is that I kept asking the question, over and over again, until I had the full answer. In this case it was easy, but sometimes it can feel aggressive, asking the same question over and over, which can leave either you or them feeling uncomfortable. But you will never be any good at interviewing people until you learn how to tolerate uncomfortable moments.
The goal is to find out if you want to hire someone, without wasting their time or yours. And asking questions like this, directly, and digging deep, is a much faster method than handing out homework assignments and then waiting a few days for them to complete it, then reviewing it yourself, then discussing it with them. And such direct, factual questions, as above, are at least as objective and as any “objective” test that you might invent.
Non-psychic candidates will struggle with this challenge, so you might find it produces some false negatives.
Since you already seem to know so much about debugging slow performing database queries on your particular RDBMS stack, though... what was your thinking in looking to bring on board someone else who duplicates that exact knowledge?
Oh, and it'll be a missing index. It's always a missing index.
The point is to start with something open ended, so the candidate can talk as much as they want on the subject, broadly, and then, based on what they say and what they don't say, I narrow in on particulars:
"Are you aware of any differences in dialect between the SQL of MySQL and the SQL of PostGres?"
It's not like they lost points for the stuff they didn't know, I simply wanted to be sure I understood the limits of their knowledge. We hired this candidate and they turned out to be great. But I hired them knowing exactly how much they knew and how much they did know.
You might want to consider if there’s a way to discover that without triggering the candidate’s imposter syndrome. Your script above reads like Tom Cruise trying to get Colonel Jessup to admit he ordered the code red. All that’s missing is the candidate breaking at the end and screaming “YOU’RE DAMN RIGHT I DON’T KNOW HOW TO DEBUG SQL”.
If they admit they don't know how to debug SQL in an interview, that is actually a positive thing. They know their current limitations and are honest about it. If they start looking for problems in irrelevant areas (like HTML) and making things up when they were specifically told the problem was elsewhere (backend), that is a bad sign.
That is exactly what I'm going for. I'm actually looking for 2 things:
* If they don't know how to debug SQL, I'd like to know that.
* Do they simply say they don't know, or do they try to bluff? Are they confident about what they don't know? Like I said in a different comment in this thread, one of my greatest hires ever was a former teacher who was very confident about what she didn't know -- she never felt any need to bluff.
Given two people of exactly equal skill, if one bluffs and the other is confident enough to say "I don't know", then I'll hire the one who says "I don't know." Because then when we are actually working together, I know they will be honest with me whenever they need help, they won't be trying to keep secrets from me.
Also, about this:
"triggering the candidate’s imposter syndrome"
Given two people of exactly equal skill, if one has imposter syndrome and another doesn't, I'll take the one who doesn't have the imposter syndrome. Or put differently, given two people of exactly equal technical skill, if one demonstrates a higher emotional intelligence, I'll hire that person.
The premise here is good, but instead of asking the same question again and again, I would rather supply more and more information until I get the answer I am looking for. (and jot down what prompting was required to get that answer) I agree with the sibling comment, otherwise this becomes a guess what I am thinking exercise.
If you wanted to check if they can write a query than ask them to create a simple select or insert statement. The drilling down turned the interview into a hostile conversation. Easily ways to test that skill.
I had an interview like this for a senior position, it turned out to be the most enjoyable interview I've ever done. The group lead really showed himself and found out at what point would I stop and say I don't know.
There's nothing aggressive about this approach unless you make it that way and finding out if people will start bullshitting or admitting the limits of their knowledge is a massive advantage to someone you might need to depend on.
"finding out if people will start bullshitting or admitting the limits of their knowledge is a massive advantage to someone you might need to depend on"
One of the best hires I ever made was a former K-12 teacher who had decided to change careers and become a software developer. She went to a bootcamp and then I hired just as she graduated from that bootcamp. When I interviewed her, I very much appreciated how clear she was about what she knew and what she did not know. She felt no need to bluff. And that foreshadowed what our communication was like once I hired her: very straightforward, with no lies or bluffing or indirections or deflections.
I think we should just jump to the end game and make interviewees do ultra man triathlons while solving differential equations. Then at the finish line, they knife fight each other to find the one we let go to the next stage in the interview. That way we can be sure who has stamina and the best competitive programming problem solving skills.
Something like squid game will truely find the A players. I mean, we wouldn’t want a bad hire after all, and everyone lies on their CV.
Right, if you're a big company you can more easily afford to have false negatives than false positives, so why not add knife fighting to the list of qualifications, just in case?
Hard disagree on this. Large companies can't afford false negatives because false negatives can hide out and move from team to team without detection. At a small company if the same thing happens it means leadership is incompetent and you have bigger problems anyway.
Large companies can and often do afford sizeable percentage of workforce making zero to negative contributions. This would be devastating for a small team with a finite runway
Indeed! But it's generally much harder to reach consensus and actually remove someone at large companies. Especially for a startup with limited runway it's existential, whereas at a large company there is far more to lose from a lawsuit than from eating a high salary as a net negative.
> But it's generally much harder to reach consensus and actually remove someone at large companies
Disagree. Big corps have well established processes for this and deep pockets to pay lawyers out of in case of any wrongful termination claims against them
At the risk of over-explaining (hopefully) obvious satire, I'm considering a "false positive" to be someone who was hired who should not have been, i.e. the hiring process gave a positive result that was wrong.
and this is your response? did you come up with extremely clever reduction in the shower and wait for the first post that's tangentially relevant (and in fact embodies the opposite of what you're attacking?)
I did the reading of existing code thing in interviews a lot a while ago (and somehow forgot about it when switching teams, this reminded me to reintroduce it), with the probably common twist of having intentional mistakes at various levels in the code, and it was absolutely great.
I feel it really gives you a good impression of the level of experience the candidate has with actual coding, and opens up for a lot of related discussion, such as security implications. Without, as the article mentions, the tediousness and artificiality of whiteboard coding (which I don't want to completely dismiss, however).
We're going to be working on a lot of code together, so it makes sense to have the interview about doing exactly that.
I interpreted squid game as an analogy for real life. If you staying alive depends on money which depends on having a job, then algorithmic interviews are a mild squid game after all.
I propose, as well, that we “weed them out” with a week of hunger games, battle bots, and rocket launch tests.
In seriousness, through a decade of experience in AI, I have found that most “famous” and “exotic” algorithms that software engineers love testing (e.g. OP’s references to sorting, tree searching, and recursion) come up exactly never in my day-to-day.
Please, limit testing to the actual data structures and algorithms that dominate 99% of the proposed work. If the proposed work is not in a field you are truly an expert at, then don’t be responsible for technically interviewing this person.
In my branch of AI, if I’m not getting solely evaluated on my ability to work with tensors, vectors, graphs, …, then I realize there is no AI group at this company (or it is inferior in its autonomy). There are so many extremely useful specializations in real-world moonshots, that would otherwise get bludgeoned upon any interview within a general purpose software engineering culture.
This would be great because if the code stinks to high heaven then I won't work there. When they think I'm not a fit for not understanding their code, the feeling would be mutual.
I agree with you in the sense that I only want to work places that recognize and value great code.
But everywhere has code that stinks. And it's hard to find people with the skills and experience to get rid if it. Before it can be gotten rid off, it must be understood.
In a sense it might be valuable to use the poor code in a reading screen. Firstly so we can agree it stinks, and the reasons why. And secondly, to find people with the attitude required to clean up messes that they didn't create (good scouts).
Even when the job is greenfield, or extending great code, I'd rather have engineers with the battle scars of doing tough maintenance.
I see a few people didn't like what you said, and that's mystifying to me because while I understand the point of view of the business owner, as well as the reality that nearly all code stinks more or less, at the end of the day your #1 obligation is to your own interests. There's no reason people should choose to work on projects they don't like if they have no financial need to do so, or if the pay isn't enticing enough.
Attitudes will change in the next decade when vulnerabilities in software and tech become more obvious, serious, and regulated. Right now we're all happy hogs feeding from the trough of few meaningful standards, but the time will come when we are blamed for human lives lost as a result of bad code, and it's no longer going to be funny that we get paid so much to write such shit. As well, doing whatever companies pay us to do will become more shameful. From much of what I read here on HN and what I've seen in my own experience, way too many software developers will do practically anything for money and prestige.
This is why the selfishness of only wanting to work with teams that write decent code is actually more beneficial for society than "fail fast, bruh". Already, the tech bruhs aren't revered so much as they were even a handful of years ago.
We need an oath the same way doctors do. We also need Software Craftsmanship principles. Unfortunately industry sucked in too many coding monkeys to make such drastic move with good pacing. Like you said - ppl have to die or be annoyed a lot to start regulating IT hard.
But TBH mission critical software has completely different set of checkboxes that are not so easy to fulfill.
The management has to be set up to allow for this. People have to be able to work towards something as a team; else we are all cleaning in different directions.
My thesis is that the true measure of reading code is still the ability to fix and extend it. So my current ideal interview problem is still a tiny toy codebase, where the interviewee is tasked with adding some (relatively trivial) feature to it. Like ten lines of code, but where those lines require you to have grokked the other couple hundred or so. Any downsides?
Only one I can think of is that a couple hundred lines (depending on content of course) can be quite a bit to grok within a 50 minute span. Especially if there are a number of places where improvements might make sense, but you have one or two in mind specifically...
I've been in that sort of position tbh, where a 200 line codebase has maybe 20-30 obviously wrong parts (creds in code, calls to 3rd party service in code w/o cacheing, maybe that could just be a service? why is this not in a queue, ...) and it's sort of weird that getting 11/14 and talking over pros and cons of the others can be considered a barely-pass.
Still, you're not wrong. Find a good enough, real-life enough example and it'd make for a great question.
You would have to fine-tune the size, taking into account that narrowing your focus to the relevant parts of the codebase is an integral part of your reading skills.
One thing I like about this (as opposed to a “blank slate” coding challenge) is that it shows how well the candidate is able to fit into the broader design style of the surrounding code. Not just the superficial things (like naming conventions), but also the structural patterns.
> fit into the broader design style of the surrounding code[, including] the structural patterns.
That might require one to be familiar with those ahead of time. If that's what you're hiring for, that's perfect of course, but I personally haven't had much experience with design patterns outside of a few C# ones in school five years ago. Yet I see myself as a competent amateur programmer (and C# isn't even my strong suit), just not one that often works on large code bases. These patterns seem like something I'd learn in a matter of days on the job... but that would not show in an interview.
It's kind of like learning a subject from the final exam.
If you can pick up on the patterns of an unfamiliar language and/or codebase and make your code look like it fits in, you're probably good at both reading and writing code, and working with a team.
I can't speak for the parent commenter, but by "patterns" I am referring to multiple layers:
Detail -- Indentation, spacing, line length, naming convention, placement of braces and parentheses.
Flow -- pacing, grouping of blocks of code, levels of nesting, when and where to break up functions into spaced blocks or helper methods.
Architecture -- class boundaries, interfaces, cross-file organization.
There are higher layers but I haven't named them yet in mind and they aren't as well tested by this kind of interview.
Fair enough, if that is meant then I imagine any competent programmer should be able to do that. I kind of figured that indentation and nesting levels were what they considered to be "superficial things [like] naming conventions". Cross-file organisation might be too large to adequately demonstrate in an interview, presuming it's not a whole-afternoon type of thing that seems popular in the USA (or at least I haven't seen them in Europe, I'm sure they must exist if only I post a comment saying they're not popular here).
I read the "patterns" part as idioms of the codebase, e.g. what a route handler looks like if this is an HTTP server, or the standard way to make an entity in a game. I think that's one of the more critical parts of the test. Certainly not GoF style "Design Patterns™".
Back when I was a hiring manager for programmers I liked setting a "code challenge" where we gave the candidate a simple game and he had to transform it into another game.
For example, we gave you a tetris game, you had to change the code to make a snake game. Or change an Othello game to a "number sliding puzzle" game.
It was very interesting comparing people that really made an effort to use the game code vs candidates that just deleted most of the stuff to implement their logic.
>It was very interesting comparing people that really made an effort to use the game code vs candidates that just deleted most of the stuff to implement their logic.
What does "very interesting" mean here? What does this particular comparison tell you? It's genuinely unclear to me which you prefer, why, and what signal you get beyond willingness to follow an instruction in a peculiar context.
I do think there is some signal based on how well the developer can utilise existing structures without getting stuck in "all you have is a hammer". That said the tradeoff is mostly time-based so if it is a take home without constraints a lot of that signal is lost.
Your comment came up as a bit aggressive for me (non native so I may have misinterpreted) but I'll try to answer in the spirit of HN:
We had candidates that were 'frustrated' because of the code. Some were frustrated that the code was low-quality, others that it used Canvas for drawing and they have never used canvas. Others about the way the logic was implemented originally.
The best candidates worked with what they had, and used the "tools" at their disposal: for example, use the code that draws a circle in the canvas already, dont try to reinvent it. At the end of the 3 hours, we talked about their experience doing the project and they had good feedback.
The bad candidates just deleted most of the code and added their own logic. My thought process is that, I could imagine them on their first day of in the team: the code startups have is never "the best one". I dont want someone who wants to throw the baby with the bathwater, but someone who can improve what we have.
A detail is that the challenge lasted 3 hours. Of those 3 hours , 1.5 hours wage candidate was "pair programming" with some developers (segments of 30 min spaced to give the candidate time to work alone). And during the pair programming , the devs were instructed to help him, give them hints and ask "revealing" questions.
You do miss out on the 'sit down and create a project' aspect though. Which shows things like familiarity with project structure, (python specific) virtual environments and paths etc.
These are good skills to have when a dev is debugging as they could get tied up for a long time dealing with something as simple as a relative path issue.
The things you mention can easily be standardized in the company, and starting a new project or setting up a dev environment should be automated to the point that you don’t even have to think about it.
>should be automated to the point that you don’t even have to think about it.
heh I like it. I'm going to put you as an intermediate skill level dev.
So these things break and need to be fixed. They also have to be setup by someone. You also need to know when something breaks, can I rule out that it's not a setup issue, a git merge that's broken a path, a user that's pushed a local dev config file accidentally.
In order to be effective, you need to have a feel and justification for the way things are. Getting someone to setup a project correctly should be a quick and easy way to do this.
For example, I've had candidates submit tech tests with code in the top level directory instead of having it in a src or some other sub folder. Which is obviously a red flag.
Yup, this is what my current take home (2 hours) project is. Read about 200 lines of code, implement about 30 somewhat complex lines in a real working project. And then there's a code review by a "junior" candidate for them to read and review, filled with about 10 bugs in 50 lines. I much prefer it to giving out algorithms questions.
By doing that you will be selecting for the "cracker" type. That is people who are able to get into any code, even under adversity, and do anything they want. It is a great skill to have even if you job doesn't involve removing copy protection, for example, he may be the only person who will be able to fix a bug in that old code base written in an obscure language when the last person to work on it left the company 10 years ago.
What you won't be testing is higher level skills. It may be easy to hack into a known codebase, and even do it cleanly, you just have to mimic the surrounding code. Getting the big idea and making long term choices is another set of skills, not exclusive with being a cracker, but not something you can see in 10 lines of code.
I've been at both ends. Building apps as a solo developer, as well as fixing some bug or implementing a single feature correctly for clients. I think it requires being able to zoom in and zoom out along the stack of abstraction that constitutes a code base, as well as either focusing on the now or a much larger timeframe.
The higher level skills can be somewhat tested by asking the candidate how it would build some module and the variation that some factors would bring in (like closer deadline, no third party service,...)
Uh, no. The skill here is one used by literally any programmer who starts work on a project they didn't start. Not that there's zero overlap, but come on.
I think there’s quite a bit of a difference between doing this very quickly and only feeling comfortable with a codebase after a while. It’s a good skill to have, but it might make low impact in certain circumstances.
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 what you are getting yourself in to. Also looking at the current open pull requests and closed pull requests to see the standard and speed of delivery. Bonus points for no PR’s and trunk driven development as that shows a very mature team.
My simpler interviews have often been with companies that have held a higher bar than the ones with tougher interviews. Those companies have often been sink or swim though and if you don’t make the grade you’ll be kicked out pretty quick. My last company had a reputation for new starts disappearing and not great that way, but the team was probably the strongest bunch of people I’ve ever worked with as only do good survived.
Trunk-based development, as I've seen it explained in the past, isn't what that site describes. The way I understood it is, no branches at all, everything committed straight to trunk, and extensive use of feature flags to keep incomplete features hidden.
Short-lived branches like that site's describing just sounds like a team that's adopted git pretty well, but not formalized usage into gitflow or similar...
Gitflow, and its various flavours, has been a popular alternative. Though it seems trunk-based is considered the preferred standard due to emphasis on achieving a stable main branch, simplified pipelines and faster cycle times. This requires a bit more maturity to get right if I'm not mistaken as you need good automation, test coverage and code review practices.
I've never heard of Gitflow or anything. I've been doing this stuff for almost a decade and trunk based with short lived development branches is all I've ever seen.
Yeah, it was pretty popular for a while but I doubt many people really do it because of it’s shortcomings. I guess if you spent most of your time in feature freezes trying to stabilise the upcoming release of your product, but I’ve often felt it’s better to either just have a few feature branches queued ready to merge once the master branch is tagged and released, or to branch master into a release branch once you have a feature freeze and only commit bug fixes to that. But then you have the annoyance of having to cherry-pick features between the branches.
It's good that people experiment with new ideas I guess, but in my 27 years as a software developer all the progress I've seen is towards continuous integration and testing. The more branches and configurations there are, the slower development is and the lower overall quality is. Sometimes there are advantages to counterbalance that, and if you've got a huge team and a huge user base it may be worth the cost. But a strong default bias towards less branches, less options, test more often, merge sooner, deploy sooner has always worked better than fancy alternatives.
Alternative is having long lived “devel” branches or even dedicated release branches. Used to be quite popular especially with waterfall style processes
Virtually all game companies use Trunk Based Development, in my experience, including many very large studios. (Outside of game engine development, which isn't really done by game studios anymore with a few exceptions)
I suspect you'll find that two of those numbers are decided by committee, one by a human, and the other by the build system.
There's good odds that it's the 26836th build, the 4045th snapshot, and the second time they've broken the file format. (or the 2nd expansion, or both)
Yea a lot of people in this thread missed that point. It's still contrived problems, just testing comprehension rather than creation.
But you can contrive much larger problems than you could ever ask a candidate to code, so you escape the ability for a candidate to practice your exact questions to some degree.
i kind of like the idea of taking them through your codebase, though. allowing the interviewer to concoct problems leads to something i'm very passionate about, and that's attempting to stifle the ego involved in the interview process. for whatever reason, intellectual prowess, and displaying it, exposes itself in interviews. the interviewer will ultimately end up trying to dominate you intellectually to satisfy their own ego. it's pretty pathetic, and rampant in whiteboarding.
Trunk driven development has worked well everywhere I've used it.
That being said, pull requests / code reviews / merge commits are totally compatible with that. Gerrit, for example, does every commit as a mini-pull-request on top of a branchless HEAD, and when the review finishes, you merge to main/trunk and off to CD it goes.
I took a job once because they were honest about the fact that the code was a complete shit show, and that’d I’d have to clean it up. I’m sure this is naive on some level, but I’d say you don’t have to lie to people. Just help them imagine doing the job and let them decide if that’s how they want to spend their time.
Bad code + time allowed to clean it up = perfectly well-defined business requirements + a license to think about code craftsmanship. That's a lot of people's dream job.
> Bad code + time allowed to clean it up = perfectly well-defined business requirements + a license to think about code craftsmanship. That's a lot of people's dream job.
I agree, also sounds like an unusually well defined role with a clear way of having impact. I would also take this job any day over another job that would bait and switch me into some rewrite death march.
But yeah, when you think about -- a sufficiently fecal-encrusted, lost-cause codebase is almost indistinguishable from an actual greenfield opportunity.
I’m sure this is naive on some level, but I’d say you don’t have to lie to people.
Sounds perfectly sensible, and not naive at all.
I'd much rather come into a situation where they were both already aware of, and entirely honest about the current shit-show status of the project -- than a situation where everyone thinks what they're doing is great and bleeding edge but when you actually take a good look at their assets, as it were... not only are they manifestly and visibly unwashed -- but no one seems to notice the stink.
> 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 your team.
The best teams I worked on who delivered working code fast, efficiently even when they are some of the most complex projects I worked on committed straight to trunk, had a very good build pipeline (super import) and worked closely together for review. The standards where extremely high yet the general feeling was it was less dogmatic, micromanaged or kept behind a gatekeeper.
The projects I’ve worked on with dogmatic pr’s generally failed to deliver anything in any amount of reasonable time. 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.
Code reviews are incredibly valuable, and are a crucial part of software development at all of tech companies. I would argue that if you aren't doing code review, then you're saying that you believe you and all your company's engineers are better than the engineers at Google, Apple, Mozilla, Microsoft, Oracle, ....
Having engineers better than the average employee at those companies is a lower bar than you'd imagine.
Smaller companies can pay much more (in expectation), and can simply poach the cream of the crop (who are invariably frustrated by useless process and corporate politics) from the big ones.
On top of that, most development processes are designed to minimize the blast radius of underperforming engineers, so your actual bar is "hire stronger engineers than the dregs from the giant shops".
No, it isn't. I know the standard of engineers at FAANGs.
Having engineers better than the competent engineers that are responsible for the dreaded process at the FAANGs, MS, etc is hard. The review policies of big projects tend to have been hard learned, and dismissing them because "they're useless process" is a poor choice.
I'm speaking based on the experience of dozens of engineers, including ones from all of MFAANG, and a half dozen almost-FAANGs.
I think you're conflating whether a process is useful for the organization with whether it is useful for the engineers implementing it. Since processes have to be uniform, you need to either figure out how to retain someone that's been at the company for a decade to continue to work maintenance jobs, or you need to figure out how to keep the fresh grads that will replace them from breaking the world.
The right choice for the organization is to keep production stable, and the bus factor high.
The right choice for the engineer is to demand massive comp increases (comparable to startup acquisition windfalls), and also the abilty to stop working maintenance.
A lot of the best engineers I've worked with used to be in a FAANG. Almost none of them would consider going back. They universally cite broken processes as the reason they left.
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 be merged without review and feedback from people across all affected subparts of the kernel, as long as they have been written through pair programming? That's a big open source example, but plenty of companies have plenty of projects for which that already applies as well.
> 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.
That is an easy trap to fall into (and wildly annoying), but it does not mean that PRs have no use outside of that.
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.
It's alright, when I started there I turned those 21 regular expressions into a single enormous regular expression that could only be understood at the time of writing, and never understood by mere humans again.
> Review also shouldn't be causing ego antagonisms.
Yeah but the tool itself is antagonistic, because it imposes a workflow of open source, a workflow that also is antagonistic with clear and absolute power. So using that tool suddenly brings that antagonism and power into a team which is supposed to be 100% collaborative, and it also only does it periodically and with random and different people in power. It's not hard to see how that becomes somewhat complicated.
Anyone who can't handle having their code reviewed before it goes into production is not welcome on my team. You can call that antagonistic or not, but I have always found that the best engineers are the ones who are able to separate criticism of their code from criticism of themselves. Get that type of people on your team, and code review is no longer antagonistic: it's just an egoless feedback process that improves the end product, which is what all members of an engineering team should be working toward.
I'm not criticising the review itself as much as rather the PR workflow and the tools of github, especially the blocking mechanism. It's my opinion that collaboration should happen much sooner rather than being pushed to the end with the review of a PR, and that you should have designated people who have the power to sign something off as production ready.
If a team inside a company want to gather inspiration from the open source world, they should look at how the owner teams of open source projects work internally, not how they work together with outside contributors.
And, it's a very common "solution" in tech to just simply not have any feelings, but people have feeling and that you can't turn off, and that's a very important contributor to work satisfaction.
There is nothing about github hat prevents you from working that way. I don't see what your issue is. But, I will say that I don't want people who can't separate criticism of their code from criticism of themselves on my team. You can certainly have whatever feelings you want, as long as it doesn't get in the way of producing the best possible product, all things considered.
Yeah there's nothing that prevents you from using a tool in exactly the opposite way that it's designed to be used, but it's also pretty unlikely that it's going to happen or that it's going to be successful.
Just make a PR early and discuss the code as you build and make changes, not sure in what way the tool wasn't made to do that. Then you also get the discussion interleaved with changes, resulting in basically perfect documentation of how the code was made and why.
The only thing PR's lack in that regard is that if the reviewer accepts it then it gets added automatically, while ideally you should re review things after the reviewer has accepted it. That way they wont accidentally accept prototype changes.
If I want early feedback on something I create a draft PR and ask for comments. I can then let people consider my approach asynchronously, in their own time.
I'm beginning to think you've never actually used github. Are you trolling?
The tool does not stop you from working in the way you suggest. Maybe it's not what the engineers who wrote it originally envisioned, but it's both simple and easy to work in the way you suggest.
> I don't want people who can't separate criticism of their code from criticism of themselves
Yeah good luck with that, nobody can completely separate criticism of their work from criticism of themselves. You are making your job as the team leader way to easy for yourself, "I only hire robots, that's how I solve all these pesky people issues".
and yet many people on this subthread alone have worked in review-required jobs, and have not had a problem with it. They've also provided reviews for other people's patches, and presumably were also able to do it without personal attacks.
It's the bare minimum of professionalism.
If you are unable to separate feedback on your work, from attacks on your person, you are lacking some fairly fundamental skills needed for professional engineering.
In over a decade of enforced code review experience, I've had one developer who was too immature to take feedback. Some folks take it personally and they shouldn't as long as the feedback is about the code. This requires some work on both reviewer and reviewee.
The guy who couldn't take feedback (person A) had code merged in that wasn't properly tested. Person B said, "hey A. I could use some help. We wrote some tests around $feature that were missing and the tests show the feature doesn't work. We see $unexpected results. Wanna make sure we agree on how this should work."
Person A: no, it works, I tested it.
B: could you help us identify the flaw in the tests then - they seem rock solid.
A: no, my code works.
B: ... ok, can you join us and talk through it?
A: no, it works.
A was removed from the team after management came in and A continued to not acknowledge his code could be wrong.
This was aberrational. We, as an org and as a team, constantly strove to keep the focus on the quality of the code. And, yes, his code was borked.
In this example, of course person A is completely in the wrong, but this is a bigger problem of being so immature that you can't admit any fault.
My suggestion is more along the lines of: use a pairing session for review so that you can bring your empathy as well as your technical expertise, and make it a step in the process just like any other steps (testing, PO approval) etc, and just trust people to do it.
I don't think there's any reason to use a tool from open source, to make code review enforced and with passive aggressive online communication and "blocking". Just seems to make work more painful, and less efficient as well actually.
Oh wow, kudos to you, not just for having to deal with that, and getting A removed, but for being so diplomatic about approaching him on the issue.
And yes, someone that lost is pretty rare, but I'd say lower-grade versions of non-transparency and making their work hard to follow is pretty typical (and frustrating).
I've had patch review as back and forth comments in email, in bugzilla, in myriad other bug databases.
If you can't send out an email with your patch as an attachment, and get feedback, then we have a problem, and the problem is not the adversarial nature of review.
But why would you use such a remote asynchronous late stage feedback loop, if you are literally sitting in the same room as your collaborators, during the whole development process?
The goal in review is to try to catch any oversights or errors in the code you wrote. The code you wrote may have been the result of of discussions with your coworkers, but the code still gets reviewed, even if by people you discussed the implementation with. That review will occasionally find things you missed, and then you can cycle, and re-review.
By definition, the review can only happen at the end of change development.
> if you are literally sitting in the same room as your collaborators
Your very premise is wrong. At any sufficiently large company, you are unlikely to be sitting in the same room as every collaborator and stakeholder, or even proxies for them.
As a simple example, the team I currently work on (on one project of several) is 10 people across 8 US cities in all four US mainland timezones, and the stakeholders and collaborators are across Australia, Asia, Europe and the Americas. A good majority of what I do is pair programmed, yet the pull request workflow is essential to letting _others_ know what is happening and why, and to allow them to have asynchronous input into the process.
You might argue it would be better for the team to be in a room somewhere. Maybe so, but the people this project demands live where they live and could not even agree a common location for that room to be if they wanted to. And it still wouldn’t help the other projects…
I was more offended by the assumption that you can simply interrupt whatever work your coworkers are doing, just because you're already in a much less pleasant open plan office.
>So using that tool suddenly brings that antagonism and power into a team which is supposed to be 100% collaborative, and it also only does it periodically and with random and different people in power.
Is code review not collaborative in your experience? In every team I've been on/run we've split code review comments between suggestions/questions and actual "I will not let this go into prod if my name is attached to it" blockers. Code review has been 99% the first set of comments and only rarely do I see anyone actually block reviews over things like style and what not that have been mentioned here.
I can't even imagine those topics getting into code review as a blocker as if we have actually strongly held opinions on mechanical issues like that, they are integrated into the various linters used so you don't even need human eyes to catch the issue.
You can have perfectly fine, high quality, peer reviewed code without PRs.
Only if some regulation requires sign offs from e.g.other depts. PRs are inevitable. In all other situations they are at best an inneficient workflow and at worst a Kafkaesque circus.
Peer programming, daily checkups, a rock solid CI, and, above all, trust in the professionalism of your team are some ingredients for high quality, high throughput software development.
This is not an opinion. It's a scientifically proven fact. As laid out in the book Accellarate.
> Peer programming, daily checkups, a rock solid CI, and, above all, trust in the professionalism of your team are some ingredients for high quality, high throughput software development.
Absolutely agree, the collaboration should happen sooner in the process, and I would add that the team should probably also have made at least a high level proposed solution together before the work even started.
Agreed. I'd like to add that PRs are not only a means to review code to improve that code. They're also way for new hires and jr devs to learn new things, how the hive mind thinks, etc. This is, ideally, today's PRs help improve tomorrow's code as well.
> 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??
Though the size and risk of a project do materially matter. If chrome ships a bug, that'll cause an impact on billions of people potentially, where as the typical software bug will impact a few hundred or dozens of people?
The way the code quality of webkit built up was by introducing a review gate. Chrome inherited its review gate from webkit, because it had established a much better track record for code quality.
The way you get any project to be "actually good" is by adopting methods to ratchet quality. Early WebKit did not have the same strict rules it has today, and we suffered because of that. Introducing review + test for every change rule was introduced, and the frequency of regressions dropped dramatically.
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.
> 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.
Well certainly I agree that naming is important. It’s up there with cache consistency and off-by-one errors as one of the two most difficult problems in computer science.
It's the opposite. Few companies know that you can do without PR. Even fewer know why they are doing PR or where it comes from. But everybody starts with PR, it's the default in their VCS UI.
I described my experience and what I saw, well I did not do any scientific research on 1000s of companies. But I still think my experience has the same validity as your statement. So it can be both at the same time, there is so many companies small and big.
Ok, let's say a lot, or most, companies use processes without knowing why.
Some might just push commits without knowing or caring about reviewing code, beyond fixing what fails in production. But others might just do "git flow" or whatever, doing thorough PRs, without knowing that the changes could be requested after being merged and without realizing the amount of time that is wasted on integrating code and re-testing.
In particular, I think that PRs come from an open source model, where you really have to gatekeep. But in a company, there is no problem in following up: the devs are salaried and you just need to put them to work on whatever is necessary.
> PR’s are great for open source projects as act as gatekeeper so not everyone can commit freely
Yeah exactly, PR's are based on the fact that you have some person who is the owner that have complete power, and many other contributors who have zero power and whose contributions will mostly be rejected. You simply don't have that situation in a company, where everyone is an owner on equal terms, and all contributions are accepted, only some with slight modifications.
So you get these really weird situations where more junior, or less skilled, people can block PR's and demand changes from other more skilled and/or senior people. And if there's disagreement in a review: the resolution of that is completely unknown, and can often blow up to a really nasty conflicts.
I have so many terrible experiences with PR's where you get hostile nonconstructive comments on your work, on github, from a person that literally sits next to you. It's the creepiest thing ever.
He's getting downvotes because he's persistently overgeneralizing from his unfortunate, legitimate lived experience to a bunch of dogmatic claims about the fundamental nature of the pull requests that contradict many other people's own lived experience.
I'm simply pointing out that the workflow of the pull request is made for a different workflow than what you normally have inside a team in a company, and it therefor quite a bad fit. And illustrating this with a few examples.
I'm getting downvoted because I'm criticising developers favorite tools that lets them pretend to be Linus Thorvalds for a moment.
No it isn't, though. It's not like there's a "right way" to run a team. The teams I've been on that didn't review code always ended up imploding because because people rationalize their innocent corner-cutting when they don't have to deal with the embarrassment of sending it off to anyone.
> Yeah exactly, PR's are based on the fact that you have some person who is the owner that have complete power, and many other contributors who have zero power and whose contributions will mostly be rejected
PRs can be approved based on two people's opinion. There doesn't need to be a central gatekeeper.
> So you get these really weird situations where more junior, or less skilled, people can block PR's and demand changes from other more skilled and/or senior people.
Sometimes junior, or less skilled people, have something valuable to say. Especially if the code could be simpler.
In a stalemate, the PR could be sent to a third party. I've suggested this many times to avoid unnecessary conflict.
I don't think it is PRs that are the issue, rather your working environment.
> Sometimes junior, or less skilled people, have something valuable to say. Especially if the code could be simpler.
Yeah and sometimes they are naive, dogmatic and overconfident, and on a crusade to change all the things! because they have read some blog post by uncle bob, and this tool is putting them in absolute power every time they do a review.
> In a stalemate, the PR could be sent to a third party. I've suggested this many times to avoid unnecessary conflict.
Ok and who might this lucky scapegoat be? I have a feeling it's not the manager for some reason..
> I don't think it is PRs that are the issue, rather your working environment.
The issue, which I'm trying to illustrate, is that the tool makes the working environment worse by introducing (hostile) dynamics between people that don't exist, which leads you into situations that you don't have resolutions for, situations that should not occur.
Using a tool that allows you to block other people's work causes unnecessary conflict in a team where people are supposed to be working together.
Edit: blocking contributions is a normal and natural thing in an open source workflow, and it is not normal and natural in a team inside a company.
I've never thought of a PR as causing conflict. For sure, you're right now that you've explained it, but as an engineer I've never felt that way.
But I'm okay with the conflict! There should be conflict at work! Ideas should be freely expressed and those ideas are going to meet contrary ones!
What wouldn't be healthy is a place where that conflict isn't resolved or doesn't lead to a better idea winning. Or where only the Lead "wins" because of their position.
There shouldn't be arguments, no one should yell or be hurt. For sure that's a bad place to work. But conflict about where to place a piece of code? Sure! Conflict about if we name it Foo or Bar? Why not?! That conflict is like the sharpening of Iron! It hurts _today_ but can strengthen and make _you_ better let alone the organization as a whole.
>But conflict about where to place a piece of code? Sure! Conflict about if we name it Foo or Bar? Why not?! That conflict is like the sharpening of Iron!
I dearly hope this is sarcasm. This is about the same level of absurdity as developers taking ages to pick project/file names. I'm not railing against a review's abilities to find bugs and make sure someone else understands. GP is right in pointing out how many fruitless review discussions exist over personal differences in what to call a function name because "I think X sounds better than Y", despite every party involved understanding the code and what it does.
I’m sorry you had such bad experience but you were working with the wrong people (feels like toxic even). PR reviews work best in environment where trust is key, and deciding together (emphasis on together) best approach to reach a goal is the main drive. In the end PRs should make you and the team stronger, as the knowledge is shared collectively.
IMO every developer should be an admin of the repo, but more importantly it’s not a gate, it’s a process that benefits communication. All the team can read and learn from small addings to the code base.
PR reviews should not equate to 100 comments either, complex discussions can (should imo) be worked out in a call discussing (dialog) best approach. Think of white boarding a problem with a fellow engineer friend.
Ofc, teams differ and some teams work better with other processes and other tools.
> I’m sorry you had such bad experience but you were working with the wrong people (feels like toxic even).
It's the tool itself and it's imposed workflow of blocking work and gaining absolute power in demanding changes, that causes the working environment to become toxic. Nobody would ever do that in a meeting "I'm blocking this work now until my demands have been met". That would be incredibly hostile, but with this tool it becomes normal.
It doesn’t have to be like this. Even when I was CTO, I’d submit my code in a PR and my team would pull me up on mistakes and inconsistencies. It was really annoying! And also great.
PRs are a great way for the whole team to learn about how the organisation cuts code, and can reduce the number of errors, but of course with poor leadership they can be used for evil.
> It doesn’t have to be like this. Even when I was CTO, I’d submit my code in a PR and my team would pull me up on mistakes and inconsistencies. It was really annoying! And also great.
I can imagine that you had fun mingling with the commoners for a day, now try it on all of your actual work, and with the whole C-level team gang up on you for each review.
> about how the organisation cuts code
You don't have any guidelines for how the organisation cuts code, and that's why you like the review process, because it covers up for that.
Why the hate? You’ve obviously had some shitty experiences but I’ve never done anything to deserve your attitude.
> I can imagine that you had fun mingling with the commoners for a day, now try it on all of your actual work, and with the whole C-level team gang up on you for each review.
You literally know nothing about me, my experiences or how I conduct my life, and rather than listen to people with a different experience to yours, you choose borderline abuse.
This is the comments section of HN. Take a break and get some perspective.
I agree with the others commenting here and can't relate to your experience at all. Review is a huge positive and while it does "slow things down" it usually is preventing people from slamming into walls at high speed, so that's a positive too.
Maybe your work experience is mostly in a high stress, prototype-heavy environment where it's more important to launch something than it is to have a maintainable, incrementally improving codebase? I worked at a consultancy like that and it was very different from "big product" long term work.
As for the social dynamics, it sounds like your workplace culture just blows.
Same here! I would have one of the more senior people review my code, and I would review his. It was a small organization and we didn't have time to do the most thorough reviews, but it was good that someone else knew something about what was going on.
"Code reviews", in general, are a good thing for knowledge transfer. If they are done for nit picking and stylistic complaints, they are not terribly valuable.
I'm confused by this discussion about development speed: are you talking latency or throughput?
Because in my experience, reviews and PRs certainly Ven damage latency, but overall throughput remains the same as it would be with subsequent follow-up fixes of these issues to the trunk.
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.
I'm personally a fan of small, clean commits, rebasing without squashing before merge, always making a merge commit, and writing a clear, through merge commit explaining what the branch does.
That way, I can treat the series of merge commits to trunk as the simple, linear overview of history, but when I'm bug-hunting I get small, clear commits to search through with git bisect.
It also means I get more useful blame output, as instead of huge hunks of a file being all from one giant bomb commit, I can see the process and evolution that the whole change went through as the original author pieced it together. That can be really helpful when dealing with obscure bugs or understanding systems with little documentation, by helping you get back more of the original author's thought process and seeing how their mental model evolved as they built the feature.
My places test suite nukes my local development environment for the full integration tests. If I am working on a hairy piece of code I open up a PR and let the CI system farm out the suite to multiple instances so I can get an answer in less than an hour.
The "right" answer is probably to refactor the test suite to be more performant, but that's never going to get approved given the amount of work that would take, and it would take me longer than I plan on being at the company to get it fixed in my spare time.
I do have it passing all tests before I try to merge if that counts?
I use the same workflow as NateEag and
mdavidn. My preference is:
• All commits SHOULD pass all CI tests
• Merge commits MUST pass all CI tests
The reason I don't require every commit to pass all tests is to maximize reviewability and logical consistency of commits.
For example, if a file needs to move, and then be modified slightly to function in its new location, I prefer to break that into two commits:
1. A verbatim move operation. This commit will fail CI but can be effortlessly reviewed.
2. Small modifications. This commit will pass CI, and its reviewability is proportional to its size.
In contrast, if you smush those two commits together, it can be hard to see where the meaningful changes occur in one giant diff. (Some tools may be smarter about showing a minimal diff for a move-and-modify commit under limited circumstances, but that doesn't always work.)
I'm with Nate here. Commits are a form of documentation and can be useful for grouping together related changes. All of this context is lost when squash merging. That said, I do aggressively rebase and amend commits on the feature branch to consolidate commits into one for each change, including any minor fixes discovered later.
For example:
When I want to see tests or documentation or config related to a change, I'll find the commit and look for other lines changed at the same time.
When I make automated changes to the code, like reformats, auto-corrections by a linter, or IDE refactors, I create a separate commit to separate mechanical changes from those that require more human scrutiny.
Commits are a form of documentation and can be useful for grouping together related changes. All of this context is lost when squash merging.
In some ways it is unfortunate that services like GitHub and GitLab have become so dominant in the industry. If you're just working with plain git there is no assumption that squashing is some kind of binary decision the way the UIs of the online VCS services tend to present it. It's normal to do an interactive rebase and squash some commits to clean things up before sharing your code, yet keep logically separate changes in their own distinct commits, and you can have a much nicer commit history if you do than with either the no-squash or squash-everything extremes. Of course you can still do that with something like GitHub or GitLab as well but I think perhaps a lot of less experienced developers have never learned how or even why they might want to.
Where I work, development is trunk based and without pull requests, and all code is reviewed. When I want to submit code, I push it to a staging area that tracks master. This causes the commits to appear in Gerrit where (conflict-free) rebasing can be performed with a single button and the code can be reviewed. During the staging I can change anything about the commits to my hearts content. Once everyone is satisfied, someone with the authority to approve the change approves it and I submit it, upon which the patches are applied as-is on the branch I staged it for.
Creating a separate branch, pushing this to your public copy of the repository and then asking someone to pull from that branch into their master branch seems absurd to me, especially if it's just 1-2 commits, and the idea of reviewing code (which I think is extremely important at team scale) should not be conflated with the concept of pull requests.
What you are describing, is pretty much exactly how it works at every company I've worked at that used PRs. You create a new branch from master, make your changes, push the branch to github/bitbucket/gitlab, make a PR. While the PR is open, you can make any changes you want to the commits in that branch (since it's just a branch).
People look at the PR (which shows the diff) and approve it if they are happy, or request changes. If there are no conflicts, you can merge with a single button. Otherwise you need to resolve conflicts somehow. I normally just rebase the branch off updated master and force push.
> Creating a separate branch, pushing this to your public copy of the repository and then asking someone to pull from that branch into their master branch seems absurd to me.
That's how it works for open source projects because people do not have permissions to make branches in the main repo, so they must fork and have the changes in a different repo. I've never seen this done at a company, I presume everyone here is talking about creating branches in the main repo and requesting review before merging the branch to master.
> I presume everyone here is talking about creating branches in the main repo and requesting review before merging the branch to master.
Regardless of whether someone is pushing to a branch in the upstream repo or pushing to a branch in a fork, the workflow is the same either way. At worst, it just means adding a remote if you want to check out someone's code locally.
I've used that workflow before, but it was called "merge requests", presumably because you're not actually requesting anyone to pull from a remote, but requesting someone to merge one branch into another.
Yeah, it's one of those things where the term everyone uses strayed from the etymology. Probably because in the GitHub UI it still calls this a pull request despite not pulling from a different remote.
Either way, if you read the thread, the folks are arguing that giving someone the power to block code getting into the mainline is bad. Judging by what you described, where someone has to approve before it can be merged, we both agree to disagree with them. We are just arguing the semantics of how that is implemented.
I don't understand at all what you're saying. Sounds like you push your code to a branch called master that tracks master but sits on a separate repo and then you apply your commits after approval via patches? Seems like you're recreating the concept of branches using repos and then using patches instead of merging.
Either way, just so you know, this kind of attitude and that you think it's a-ok to use such a convoluted process for what is effectively the same thing would 100% mean me not hiring you. That may not mean much on some random forum on the net, but you will encounter it a lot as the industry is definitely not aligned to this odd flow you described.
I'm not sure how you read what you're suggesting into my description. I make a commit.
git commit
# ...
git commit # or two
I push them to the staging area for a given branch, in this case master
git push HEAD:refs/for/master
This isn't a branch in the traditional sense but git conveniently sees it that way.
Now they're immediately available for review. That looks something like this (this is not from my workplace, but the Go team's Gerrit instance): https://go-review.googlesource.com/c/go/+/361916
> Either way, just so you know, this kind of attitude and that you think it's a-ok to use such a convoluted process for what is effectively the same thing would 100% mean me not hiring you.
That's fine. There are plenty of companies that manage to hire based on competence, experience and references so I'm not exactly aching to get hired at a place that would deny me for having used Gerrit.
It really seems like this "no PR" idea is having a moment on HN. I can't tell if it's just engagement bait contrarianism or genuine. That any developer would trust themselves even to just not make simple typo's is baffling to me.
Maybe the kernel of a good idea in the comment: A team that prioritizes reviews, such that PR's are not a significant hurdle, is indeed a sign of a mature team.
This is precicely why only few companies will do this kind of interview.
Good candidate would instantly notice the team is being labeled as seniors but instead are a bunch of crappy devs with years of experience.
Code does not lie. If you are crap you gonna produce crap code.
Ive recently hit such a mine. Lies during interview. Gonna sit here through vacations and jump the ship. Was sold on working with experienced ppl with over 10y of experience. Turns out those ppl IMHO have less skill than me few years ago with only 2y experience. Legacy bugers that did not improve over the years.
I’ve never understood this. I’ve had Teams grill me on questions I know most of them wouldn’t pass and they themselves said they’re struggling with delivering things. Some weird dick measuring thing
If you're on a team that isn't currently having staffing issues, I don't think it's weird to want your next hire to be better than the median person on the team. "Raise the bar," etc. Asking harder leetcode questions seems like a bad way to achieve it, but I get the impulse. If the bar for member nine of the team is approximately "better than the bottom 4" then that 9th person in is likely to have at least one moment of "wait, how did you pass that interview?" at some point after they come on.
Or they don't have the time or manpower to fix/learn things and
are trying their best to hire someone to help right the ship before bringing you onboard. Such a negative take.
One who needs a good, steady paycheck at a stable company? Just because hiring team doesn't have time to learn/apply a specific piece tech on the job doesn't mean they're low performers or less competent than the interviewee. They can be sufficiently busy with their current jobs and need someone experienced to lay the groundwork for others to run with. People that go and implement stuff when they're already busy usually half arse it. Those people are just creating issues that'll be fully realized months, years later.
If you say you know tech X, manager wants to hire for tech X to solve a business need, and I'm asked to help interview, I may ask you questions about things I don't know or do myself because it's my professional responsibility to my employer and manager to help them hire a quality candidate. Maybe some people use the interview as a place to be jerks or show off but that's not always true. Personally I'd rather not be involved in the process at all.
Last person I hired to work with me in my previous job I walked them through the entire code base, architecture and CI setup. This gave the hire a chance to best understand day to day work and me a chance to know which parts they understood immediately and which they would need to learn. Works perfectly for me.
> despite being grilled on it and the level of skill not to great they just want senior people
Happens to me too, I get tough interviews, and a good salary, but the work is exactly the same as the juniors "a guy on the scrum team". I don't even understand why they hire seniors, or have that title when it means nothing. I guess they expect them to be just faster versions of juniors.
yes and no. the last company I worked at is a feature factory. They have senior, and staff engineers, on various product teams that have 0 technical influence. What they expected is you to somehow increase quality of the product via code review and writing design documents... but the quality of the product was being destroyed by a handful of people making all of the technical and architectural decisions and forcing their platform on the rest of the org. Upper management doesn't have the technical expertise to see this and thinks this approach will lead to some sort of unified performance and scalability. Maybe it will down the road, but if I can't change out the underlying database, ORM, or web framework for something that is just _better_, I'm ultimately just working on the assembly line at the feature factory as a Sr/Staff engineer.
I think we must've worked at the same place. I took a job at a jira feature factory working on an aging, ossified code base. Literally, it was using the same tech I was using 10 years ago, almost exactly, except it was new back then.
I spent most of my time writing design documents, doing code reviews, and shepherding my relatively small changes through review. Something I could've banged out in a week would take months due to all the review and processes.
It would be quite absurd to dismiss a candidate based on the editor and yet I find it more important that a particular cloud provider, which quite often is a requirement.
> Those companies have often been sink or swim though and if you don’t make the grade you’ll be kicked out pretty quick. My last company had a reputation for new starts disappearing and not great that way, but the team was probably the strongest bunch of people I’ve ever worked with as only do good survived.
This is not what I would call a "strong" team. This is what I would call a developmentally stunted team. By that I mean they've reached competency as individuals, but they have limited ability to level up fellow developers. As such, they are not a producer of talent and rely on actual strong teams for their hiring pool. In my experience, these teams are best avoided if you actually care about growth.
I've been doing this for a long time now. I present the candidate with a function (on paper) that compiles and runs. It does the job but it does everything poorly: an embedded connection string, leaves the connection open, no error handling, bad variable names, no comments, etc... EVERY candidate can find something wrong with this code and the things they pick up is informative. Juniors can find the easy stuff, more senior-level folks find the deeper flaws about the basic structure of the code like figuring that a lot of this is boilerplate that could be implemented elsewhere.
I always preferred this style of technical interview because it has two critical advantages over tests based on coding from scratch in an interview situation.
Firstly, you can work with realistic code. It can use your real tech stack and there can be enough code with a realistic structure to see how a candidate finds their way around.
Secondly, you can make it open-ended. For junior candidates it might be best to stick to simple questions like what does this short function print and see if they can reason through some basic logic. But for more experienced candidates it can be a general code review.
Your example code can include anything from superficial mistakes like typos and unnecessary duplication to strategic problems like inflexible designs or inconsistent models. You can include obvious or subtle logic errors and performance problems appropriate to the level of the role.
Ask each candidate to talk you through what they're thinking about as they read the code and see what level they work on and how much they find in the time available. Are they methodical? Do they flag the trivial stuff but not get hung up on it? Do they spot the O(n^2) algorithm? Do they spot that algorithm but then start talking about worst case vs amortized performance and using a profiler to decide whether changing to a theoretically better but much more complicated algorithm would be justified?
In this kind of environment you can quickly tell if you have an awful or outstanding candidate. For the majority who fall in between you have an objective basis for comparing their performance. And all without any trick questions or LeetCode junk at all.
When I did interviews @ Google (I only do hiring committee work now, thank god :-) I usually asked questions around bugs I added to an existing codebase, to see if the candidate can avoid the pitfalls I ran into by making bad assumptions.
As I'm a pure C coder, my starter question was usually something like:
a). What does malloc(0) return ?
b). Why does it do that ?
I'm not anything other than a C tourist, and I see that the man page says it returns either NULL or a "unique pointer value that can later be successfully passed to free()."
I'm kind of at a loss about why it can return either of those two things, somebody want to take a shot at explaining it?
Both NULL and a unique pointer value can be safely passed to free() :-).
Answers to this question taught me about the candidates taste and understanding of good API design :-).
Both NULL and "unique pointer" are correct answers, but all modern implementations only chose to return one of these. My follow-up question is "why ?" :-).
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 realloc() which may favor NULL or a non-null pointer to 0 bytes, but that's too much work to figure out now.
Suppose that malloc(0) returns a not-NULL value. Is malloc(0) == malloc(0) guaranteed to be false? (I think that it is, which is how ENOMEM can happen.)
So, the "right" answer is probably malloc(0) returns a unique pointer because then the error check is simpler - a NULL return value is always a true error, you don't have to look at size.
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 zero size passed in ?
glibc and all implementations distinguish the two by allocating a internal malloc heap header, but with internal size bookkeeping size of zero, returning a pointer to the byte after the internal malloc heap header.
The only valid things you can do with the returned pointer is test it for != NULL, or pass it to realloc() or free(). You can never dereference it.
Returning a valid pointer to NO DATA is what all modern implementations do when a size==0 is requested.
In an interview situation, discussions around all these points are very productive, telling me how the candidate thinks about API design and how to fix a bad old one, whether they know anything about malloc internals (which is essential as overwriting internal malloc header info is a common security attack), and how they deal with errors returned from code.
Remember, it was only my warmup question :-). Things get really interesting after that :-) :-).
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.
malloc(0) != malloc(0) because the internal malloc header is different for each allocation.
Asking for a malloc of size n, means internally the malloc library allocates n + h, where h is the internal malloc header size. So there is always an allocation being done for at least a size of h, just with an internal bookkeeping "size" field set to zero.
and yes, while(1) malloc(0); will eventually run out of memory, counter-intuitively :-).
> I'm not anything other than a C tourist, and I see that the man page says it returns either NULL or a "unique pointer value that can later be successfully passed to free()."
>
> I'm kind of at a loss about why it can return either of those two things, somebody want to take a shot at explaining it?
Any return from malloc, whether it succeeds or not, is a valid argument to `free()`. Hence, it can return NULL because `free(NULL)` is legal, and anything other than NULL has to be a unique pointer, because if it returns a duplicate then calling `free()` no the duplicate will crash.
I find questions like this lack an on-ramp. Either you know or you don't. If you don't it gives you 0 indication of the skills.
I once flunked a faang interview because the interviewer mispronounced (or used the correct pronunciation I was unfamiliar with) of "arp protocol"
I had no idea of what was being asked and was racking my brain for something I didn't think I had ever used to down every computer in the library of my high school 12 years before.
No, that's not true. If you know anything about C and writing secure code (which I what I was probing for) you know about using malloc(). You know because you have to know something about the internals of memory management.
Imagine you just read a 4 byte value off the network, and it's part of a protocol that specifies how many more bytes there are to read. You might (in error, ahem... :-) pass that value to malloc(). So knowing what might happen if an attacker puts an unexpected value in there is something you need to think about.
If you don't know or can't guess because you don't know how malloc() works, then you're not the person I'm looking for.
That's one way to look at it. Another might be that if you write code that passes unsanitised input to anything and can get that code through testing and review then maybe you're not the kind of organisation that a candidate who knows about security wants to work for.
In the end this is still a language lawyer question. It's a technicality that should never be relevant. If it is you've already gone wrong several times. In other languages there might be an argument that it probably does something reasonable and any developer experienced with that language should be able to make an educated guess about what that would be even if they don't know. But you asked about C, a language infamous for having undefined behaviour in many such situations, so I don't think even that is a particularly compelling argument here.
Many protocols read values of a network to specify how much is left in the packet (it's how packet boundaries are usually encoded, specifically in SMB1/2/3). So yes, no matter how paranoid you are you're eventually going to have to pass that value to something in your code :-).
Many protocols read values of a network to specify how much is left in the packet (it's how packet boundaries are usually encoded, specifically in SMB1/2/3).
Sure. So do many other protocols and file formats. But if you're using those values for memory allocation without checking them first then getting a 0 might be the least of your worries. Unchecked large values might be a DoS attack waiting to happen.
If you work with C code where security is a factor then surely you already know this so it still seems odd to put so much emphasis on your original question. You do you I guess. :-)
It's just a warmup. Tells me how the candidate thinks about such things. In production code of course the max size is limited to avoid DoS. My bug in Samba was missing the behavior of the zero case.
Unless you are specifically hiring for low-level network performance tuning (which is not how 99% of Googlers are hired), this still seems like a trivia question that's only marginally related to a person's C(++) competency. My impression is that Google discouraged asking such questions.
I don't code to in C++ anymore. Did a long time ago. I'm looking for careful programmers around security and API design. This is nothing to do with performance tuning. It's to do with security.
According to GNU libc, malloc(0) can return either NULL or a valid unique pointer value which can later be passed to free(). You seem to imply that one or the other is definitely true.
Firstly, you seemed to imply that malloc(0) always, for all implementation, returns one or the other, and one of the two answers was wrong and the other was right.
Secondly, why should a C application developer know enough about the implementation details of malloc() to answer such an esoteric question? Malloc can not, by definition, be implemented in C, so it seems a bit out of scope.
Malloc cannot, IIRC, be implemented in standard C only. It needs non-standard system calls. Specifically, how does malloc() itself allocate new memory?
I mean, this is just being pedantic, but you could in theory declare a giant uninitialized non-local array
uint8_t my_memory[4*1024*1024*1024];
and some additional globals for housekeeping[1], and use that as your backing memory for your malloc implementation written entirely in "standard C"[2]. And that usually does not even waste any memory, since my_memory will be in the zero-filled BSS section and the actual backing pages only allocated on demand by the OS once you write on them for the first time.
Of course in reality there's probably a few "non-standard"[2] system calls in the path, mostly mmap.
[1] Not necessarily, you can place them at special places in your my_memory array by convention, e.g. at the start of the array.
[2] I assume that we're excluding standards on other layers like POSIX, otherwise it's trivial.
IIUC, brk and sbrk have been removed from POSIX, and using mmap() just to allocate memory is a pretty weird way to use mmap(), and probably not what you would want for a implementation of malloc().
"Traditionally, allocators have used sbrk(2) to obtain memory, which is suboptimal for several reasons, including race conditions, increased fragmentation, and artificial limitations on maximum usable memory. If sbrk(2) is supported by the operating system, this allocator uses both mmap(2) and sbrk(2), in that order of preference; otherwise only mmap(2) is used."
Also, Google's tcmalloc uses mmap to get system memory:
"An allocation for k pages is satisfied by looking in the kth free list. If that free list is empty, we look in the next free list, and so forth. Eventually, we look in the last free list if necessary. If that fails, we fetch memory from the system mmap."
In fact I'd be surprised to see a modern malloc implementation that doesn't use mmap under the hood.
I did this. I had a folder with a few printed sheets of code hiding some slightly tricky C/C++ bugs from our codebase. I would ask the candidate to see if they could identify the issue. I don't think anyone ever actually got them but at least I got an idea of their thought process and experience. So mixed results.
Of course, if a developer comes with experience and personal references from acquaintances, you don't need to test their abilities at all during the interview process.
For example, I hired a backend developer last month. She already had the job because she came highly recommended from a trusted friend of mine, but she didn't know that. Here's how the interview went down:
Me: I see on your resume that you've achieved Grand Master level in Microsoft Solitaire Collection.
Her: Yes.
Me: Well, we won't waste any more time then. Welcome to the team.
Completely off-topic: I am 99% certain that I read a very similar comment on HN a few weeks back. Did you happen to share this anecdote on another HN thread?
Same here...except I tell up front when they apply that it's more of a formality for HR. I respect their time and don't want them to waste time preparing or getting nervous. I tell them it's mostly for the team to get to know them and for them to evaluate the team and we'll keep the interview light.
I still give my team the option to decline them if there's major red flags - but I have not had that happen.
Also, a reminder...an interview it is a 2 way street. Your 1 question interview doesn't give the candidate an opportunity to interview you or your team and the "Welcome to the team" is a little presumptuous...I assume it was left out for dramatic effect and I'm sure (I hope?) you allowed the interviewee time to ask questions.
I still think Triplebyte gave me the best interview I've experienced, and I've done ~100 over the years at all kinds of companies. I've also interviewed people ~1000 times myself.
It was even more impressive because the person doing the interview wasn't super competent and yet they still managed to do a fairly comprehensive technical evaluation in ~2 hours.
One section was a debugging session where you had to get tests to pass. The code and tests were quite decent, which made it quite easy to show off. Every company should do this.
Too bad the Triplebyte promise (not having to do phone screens) was a joke and their business model wasn't good, but that part of Triplebyte had major promise.
Reading probes the most fundamental skills. Reading code is probably 95% of what a developer does as part of their job.
This is true. Most of the time I'm only reading my own code, and over the years I've been motivated to code better by having to go through crap I wrote early on.
In my jobs I am reading other peoples code most of the time. Maybe a consequence of small company/team “jack of all trades” culture and also there are more “not mes” than “mes” contributing to the code base. Hell someone doesn’t even need to be currently employed there for them to have the pleasure of me reading their code (or maybe my pleasure!)
564 comments
[ 3.0 ms ] story [ 337 ms ] threadas a more concrete example, as I get older I'm less interested in working on the computer off hours. I've noticed I don't even play video games as much as I used to. I actually started doing wood working a couple of years ago, and playing with 3d printers. It does go in fits and starts though - sometimes I get an itch to play with something just for fun...
That said, I do find this hard in interviews (when asked about past experience) and prefer to just be given a web application riddled with every type of bug and try to bingo them all in the time given to show that I understand them all. That one was my favorite interview. But that might just be a different style of interview for a different type of job.
Then we gave an exercise* to write some code. In my opinion a junior (but not noob) programmer should be able to do it: the code should loop through a file that is too large to keep in memory and collect some statistics -- e.g. look up numbers associated with keywords and compute the average (where the keyword list is the large file; the lookup table is like ten entries).
This candidate (like the last one) instead worked around the memory constraint and just split up the file using external software such that they could fit the whole thing in memory. Not combining the partial results later or anything, no, just compute answers based on this one slice. Then in the interview afterwards, they could not describe how this could have been done differently, also not with hints (I understand they're under pressure during the interview). The code also had other problems (e.g. opening and re-reading this lookup file inline in the main loop... which runs a billion times). Even though functional (if not always giving the correct result; mistakes happen), they had neither a concept of memory complexity nor computational complexity.
So that's just anecdotal, maybe it was an outlier. From my perspective, talking about projects and code isn't always the same as them being able to work on the problems you're hiring them for.
* Note this was not entirely my choice, even though with hindsight it was a good idea we should at least have compensated their time regardless of the outcome imo.
Doing it because you choose to. Because you like it. That's key.
a) outdated, because almost all work relevant to my subfield have been at my job for a some years now (and my experience has grown accordingly, since), or,
b) irrelevant, because by now my personal projects are mostly orthogonal to my job (e.g. stuff involving electrical engineering and digital signal processing), which I am absolutely not applying for.
An example would have been nice though, as I'm not sure how to find a piece of code that does something standalone that is too large to grasp in 20 minutes yet make a reasonable prediction at the output. That combination seems kind of weird.
I wonder how well it would work to modify OP's idea and present a candidate with some code that has a bug in it (you reveal what the bug is to save time, they'll need to understand the code anyhow) and you ask them how they'd fix it. First broadly, like what approach, then actually write part of it.
To take my own advice and make the discussion more concrete, I should add an example. Random script from my github: https://github.com/lucb1e/randomdirectory/blob/master/random...
To prepare for the interview, one could add an argument like -h to show usage (in the same style as -v is currently implemented), then tell the candidate that the bug is that passing -vh executes neither -v nor -h (one would commonly expect it to execute both). Fixing this requires a 'structural' change to this little bit of code, but it's small enough to easily grasp and implement. The candidate might propose to loop over each character after a single hyphen to fix this, or they might propose to throw out this custom reinventing-the-wheel and use a proper arg parsing library. Either is fine, but they should be prepared to write the code (in any language, they can write just the changes in C, Python, JS, or whatever language they're comfortable with).
Has anyone tried such an interview coding question?
In my opinion, you should be free to look that up: that's what you'd do on the job as well. I also google it every time I need to use arg parsing in literally any language because I need it fairly infrequently, and frankly that's how this code came to be: too lazy to look it up again, too easy to write myself, and enjoying the writing of code. Any other solution would also be fine, this test would be about seeing if you can read and write code rather than if you come up with the (what the interviewers think) is the perfect solution.
[1] And if they do, we probably end up pulling up the man page for them because we don't remember either. It's just not the relevant part of any coding interview, the reading or the writing kind.
That's amazing! I didn't know that our little project is used in interviews. Happy to hear that you liked working with the code base!
The project was:
- you have two files: listings.json and products.json
- listings.json lists ~20,000 Amazon listings for electronics with fields like title, brand and price
- products.json lists ~1,000 digital cameras, with fields like brand, family, model
- your job is to write a script that, for each entry in listings.json, emits the best match (or no match!) from products.json
Your solution could be as naive or as fancy as you wanted. The main point was to have something for the next stage.
We'd run your submission, and use that to show you some false positives and false negatives. Then we'd ask you to debug why each false positive or false negative happen, explain it to us, propose how you'd fix it, and identify any trade-offs in your proposed fix.
Eventually, we wanted to offer a non-take-home-project interview. We already had a bunch of existing solutions from employees, so we used those to run a stripped down version of the interview that just focused on the code reading/debugging/proposing fixes part.
I think both of these interview approaches were pretty effective at giving candidates a natural environment to demonstrate skills that they'd actually use on the job -- debugging, collaboration, predicting downstream implications of changes, etc.
We were brainstorming what type of content would make fun and educational workshops for students. I proposed something along OPs lines. I called them "debugging minichallenges". The idea is to present students with a buggy implementation for a simple problem, and they need to find and fix the bugs. Just as OPs argues, the concept is that reading and understanding code is a fundamental skill.
I have the set of sample implementations in my Github, for example the one in Python [1]. This is our first time running this workshop so I can report how well it did after the event.
[1] https://github.com/angarg12/minichallenge-flood-fill-python
[1] https://github.com/angarg12/minichallenge-flood-fill-python/...
> I can quickly train a person to have knowledge in some domain,
This part I have a little bit of trouble with. If you have a trivial domain, sure. In my experience domain knowledge takes a while to truly get your head wrapped around. But I expect that's why there's always other things that people do in an interview.
I'm currently looking for someone to join our own team, I'll be taking this adjusted priority in mind.
So far I've had really positive feedback from job candidates. A couple even described it as their favorite interview ever! I feel like it has worked well, given the people we have ended up hiring.
List as many different ways as you can for how to figure out if a singly list is broken and has a loop
Hmmmm, really?
I feel like the answer to those questions determines which is faster. One may say that you only compare (lines_read / time_reading) and (lines_written / time_writing) to find an answer, but if we remove all "time spent thinking" from both then it kind of feels like a meaningless comparison.
> [...] That said, I do find [it] hard in interviews [to talk] about past experience[s] and prefer to just be given a web application riddled with every type of bug and try to bingo them all in the time given to show that I understand them all. That one was my favorite interview.
Now I see johnqian suggesting to ask interviewees how to best see their skills, and it sounds great. If the interviewer doesn't have something like this on hand, we could still go into different types of bugs that they care about.
I've also had interviews that went terribly, particularly when no technical details were asked at all for a technical position (e.g. a manager just asking the standard questions where you need to give platitude answers, such as "why are you specifically excited to intern with Vodafone", methinks: "because I want that degree and you were the only option I could think of within 45 minutes commuting"... but one can't say that so I made something up.. he probed further on that... it went downhill from there), so being able to steer away from that sounds great.
But it might also be that I would feel unprepared for such a question and give a total non-answer, helping neither party.
Asking this in an email ahead of time might be better, though then the effect you're suggesting might be even stronger.
Edit: I'm not the downvoter, to be clear. I think you raise a good point even if I am not sure whether it would necessarily be that way.
As for avoiding unconscious bias, was that ever an option?
I mean, forgive me for stating the obvious, but they're not trying to make you come out of your shell because they want a friend to talk to.
There's no need to be demeaning to people (not just "girls") who work in HR.
Seems like those interviews are doing their job then if they screen you out
That's exactly what it is, and it's important. It's not helpful over the long run if we hire someone who can crank out good code, but their mindset negatively infects the rest of the team, causing morale to drop and people to leave.
I give technical interviews, but I'm also evaluating soft skills while I do it. I would rather have a team with ok-to-good technical skills, than a team with a rockstar prima donna who sabotages the cohesiveness of the whole.
Very little empirical evidence supports it, despite people parroting how "useful" it is. You're also teaching people to BS about themselves to get a job, perverting the entire thing.
If you want to psychologically analyze people, use a method which is actually supported empirically instead of the "do what every other pseudo-psychologist does" method. Big 5, for one, actually has some empirical evidence supporting it, but is barely ever used.
On the other hand, if you can actually figure people out in an hour under a single set of constraints, somehow being able to extrapolate that to the work environment as a whole, while also trying to put the work environment in a much better light than it actually is: quit your IT job now and make millions selling books.
I like leading with that, and am a big fan of the initial "what's an interesting problem you had to solve?" question, but I made the unfortunate experience that even people with good war stories are sometimes very shaky in the basics of the particular subfield I'm interviewing for. To the point of deeming someone not a good match for our team, but encouraging them and internal recruiters to interview with other teams.
There is ultimately no full substitute for getting down to brass tacks and actually do some "sample work" together. Whiteboard coding is the usual (and, I guess, often poorly implemented) approach, but I've used reading existing code, with added intentional mistakes, to great success so far.
EDIT: Just read your new reply in a sibling thread that you're doing that anyway, and project and war stories are supplemental. It seems like we're in agreement, then!
> gives a chance for the candidate to ... put forward their best self.
How does that work? If it genuinely won't be evaluated, what's the point of getting their best self? How do you make sure things you learn in this don't affect your judgement later?
Yeah I've also been using this approach for Fullstack Devd: A small page with a bit of CRUD + a small ticket description of what the page is supposed to do.
The code contains various bugs or questionable implementations, the interviewee is supposed to analyze the code and to either fix the issues right away or to write comments.
Nobody is expected to get everything right within the time slot, but I've found it to be a great test on how a candidate might perform in their day to day work.
Here’s a real-life example. I asked a recent applicant (for a fullstack software job, where we were hoping to hire a novice-to-mid-level engineer):
Me: How would you improve a situation where a page is loading slowly and you suspect the problem is related to the database?
Applicant: Well, I’d start by checking the HTML, is it correctly done, and then the CSS, is there any redundancy? And then the Javascript, is it correctly written, is it minified? Can we speed that up at all? Check the timeline, the API calls, see what is slow.
Me: Okay, great, that’s a good start, but what else? If the problem is not on the frontend, then what?
Applicant: Uh, well, then, I guess I need to look at the backend database model code. Is my database model code concise? Am I fetching the data needed, without any excess?
Me: Okay, great, that’s a good start, but what else?
Applicant: Uh, what else? Well, uh, we really need to look at that database code. Is the model bloated? Can we slim it down?
Me: Yes, okay, you basically said that already, anything else?
Applicant: Uh, well … uh, you need to check the HTML and the CSS and the Javascript and then, uh … API calls … uh ... the model code, make sure that is cleaned up. That needs to be lean.
Me: Yes, okay, but you said all of that already, anything else?
Applicant: Uh … well … the model code … and uh …
Me: Have you ever worked directly with a database?
Applicant: Uh … not much?
Me: If you get unexpected results from your model code, do you know how to debug the query?
Applicant: Uh … I guess I could … not really.
Me: Have you ever looked at the “slow query” log?
Applicant: Uh … no?
Me: Do you know how to run EXPLAIN or ANALYSIS?
Applicant: Well … uh …. no.
Me: Have you ever written SQL by hand?
Applicant: Uh … no.
Me: Are you aware of any differences in dialect between the SQL of MySQL and the SQL of PostGres?
Applicant: Uh … no.
Basically, they were somewhere between a novice level and a mid-level engineer, so they knew the frontend reasonably well, but they didn’t know a thing about databases. Which was okay, because that was what we were looking for. We still hired them and they turned out to be great in some areas, and they were eager to learn about the things they didn't already know. But obviously, if I'd been hiring a senior-level engineer, and it turned out they knew nothing about databases, that would have been a problem. The crucial thing is that I kept asking the question, over and over again, until I had the full answer. In this case it was easy, but sometimes it can feel aggressive, asking the same question over and over, which can leave either you or them feeling uncomfortable. But you will never be any good at interviewing people until you learn how to tolerate uncomfortable moments. The goal is to find out if you want to hire someone, without wasting their time or yours. And asking questions like this, directly, and digging deep, is a much faster method than handing out homework assignments and then waiting a few days for them to complete it, then reviewing it yourself, then discussing it with them. And such direct, factual questions, as above, are at least as objective and as any “objective” test that you might invent.
Non-psychic candidates will struggle with this challenge, so you might find it produces some false negatives.
Since you already seem to know so much about debugging slow performing database queries on your particular RDBMS stack, though... what was your thinking in looking to bring on board someone else who duplicates that exact knowledge?
Oh, and it'll be a missing index. It's always a missing index.
"Are you aware of any differences in dialect between the SQL of MySQL and the SQL of PostGres?"
It's not like they lost points for the stuff they didn't know, I simply wanted to be sure I understood the limits of their knowledge. We hired this candidate and they turned out to be great. But I hired them knowing exactly how much they knew and how much they did know.
* If they don't know how to debug SQL, I'd like to know that.
* Do they simply say they don't know, or do they try to bluff? Are they confident about what they don't know? Like I said in a different comment in this thread, one of my greatest hires ever was a former teacher who was very confident about what she didn't know -- she never felt any need to bluff.
Given two people of exactly equal skill, if one bluffs and the other is confident enough to say "I don't know", then I'll hire the one who says "I don't know." Because then when we are actually working together, I know they will be honest with me whenever they need help, they won't be trying to keep secrets from me.
Also, about this:
"triggering the candidate’s imposter syndrome"
Given two people of exactly equal skill, if one has imposter syndrome and another doesn't, I'll take the one who doesn't have the imposter syndrome. Or put differently, given two people of exactly equal technical skill, if one demonstrates a higher emotional intelligence, I'll hire that person.
There's nothing aggressive about this approach unless you make it that way and finding out if people will start bullshitting or admitting the limits of their knowledge is a massive advantage to someone you might need to depend on.
"finding out if people will start bullshitting or admitting the limits of their knowledge is a massive advantage to someone you might need to depend on"
One of the best hires I ever made was a former K-12 teacher who had decided to change careers and become a software developer. She went to a bootcamp and then I hired just as she graduated from that bootcamp. When I interviewed her, I very much appreciated how clear she was about what she knew and what she did not know. She felt no need to bluff. And that foreshadowed what our communication was like once I hired her: very straightforward, with no lies or bluffing or indirections or deflections.
Something like squid game will truely find the A players. I mean, we wouldn’t want a bad hire after all, and everyone lies on their CV.
(Obviously /s)
Disagree. Big corps have well established processes for this and deep pockets to pay lawyers out of in case of any wrongful termination claims against them
and this is your response? did you come up with extremely clever reduction in the shower and wait for the first post that's tangentially relevant (and in fact embodies the opposite of what you're attacking?)
I feel it really gives you a good impression of the level of experience the candidate has with actual coding, and opens up for a lot of related discussion, such as security implications. Without, as the article mentions, the tediousness and artificiality of whiteboard coding (which I don't want to completely dismiss, however).
We're going to be working on a lot of code together, so it makes sense to have the interview about doing exactly that.
In seriousness, through a decade of experience in AI, I have found that most “famous” and “exotic” algorithms that software engineers love testing (e.g. OP’s references to sorting, tree searching, and recursion) come up exactly never in my day-to-day.
Please, limit testing to the actual data structures and algorithms that dominate 99% of the proposed work. If the proposed work is not in a field you are truly an expert at, then don’t be responsible for technically interviewing this person.
In my branch of AI, if I’m not getting solely evaluated on my ability to work with tensors, vectors, graphs, …, then I realize there is no AI group at this company (or it is inferior in its autonomy). There are so many extremely useful specializations in real-world moonshots, that would otherwise get bludgeoned upon any interview within a general purpose software engineering culture.
But everywhere has code that stinks. And it's hard to find people with the skills and experience to get rid if it. Before it can be gotten rid off, it must be understood.
In a sense it might be valuable to use the poor code in a reading screen. Firstly so we can agree it stinks, and the reasons why. And secondly, to find people with the attitude required to clean up messes that they didn't create (good scouts).
Even when the job is greenfield, or extending great code, I'd rather have engineers with the battle scars of doing tough maintenance.
Often times it requires not only a lot of skill but also a lot of work.
I can do it but be prepared to pay me 2-3 times the normal wage.
Attitudes will change in the next decade when vulnerabilities in software and tech become more obvious, serious, and regulated. Right now we're all happy hogs feeding from the trough of few meaningful standards, but the time will come when we are blamed for human lives lost as a result of bad code, and it's no longer going to be funny that we get paid so much to write such shit. As well, doing whatever companies pay us to do will become more shameful. From much of what I read here on HN and what I've seen in my own experience, way too many software developers will do practically anything for money and prestige.
This is why the selfishness of only wanting to work with teams that write decent code is actually more beneficial for society than "fail fast, bruh". Already, the tech bruhs aren't revered so much as they were even a handful of years ago.
But TBH mission critical software has completely different set of checkboxes that are not so easy to fulfill.
The management has to be set up to allow for this. People have to be able to work towards something as a team; else we are all cleaning in different directions.
I've been in that sort of position tbh, where a 200 line codebase has maybe 20-30 obviously wrong parts (creds in code, calls to 3rd party service in code w/o cacheing, maybe that could just be a service? why is this not in a queue, ...) and it's sort of weird that getting 11/14 and talking over pros and cons of the others can be considered a barely-pass.
Still, you're not wrong. Find a good enough, real-life enough example and it'd make for a great question.
That might require one to be familiar with those ahead of time. If that's what you're hiring for, that's perfect of course, but I personally haven't had much experience with design patterns outside of a few C# ones in school five years ago. Yet I see myself as a competent amateur programmer (and C# isn't even my strong suit), just not one that often works on large code bases. These patterns seem like something I'd learn in a matter of days on the job... but that would not show in an interview.
If you can pick up on the patterns of an unfamiliar language and/or codebase and make your code look like it fits in, you're probably good at both reading and writing code, and working with a team.
I can't speak for the parent commenter, but by "patterns" I am referring to multiple layers:
Detail -- Indentation, spacing, line length, naming convention, placement of braces and parentheses.
Flow -- pacing, grouping of blocks of code, levels of nesting, when and where to break up functions into spaced blocks or helper methods.
Architecture -- class boundaries, interfaces, cross-file organization.
There are higher layers but I haven't named them yet in mind and they aren't as well tested by this kind of interview.
For example, we gave you a tetris game, you had to change the code to make a snake game. Or change an Othello game to a "number sliding puzzle" game.
It was very interesting comparing people that really made an effort to use the game code vs candidates that just deleted most of the stuff to implement their logic.
What does "very interesting" mean here? What does this particular comparison tell you? It's genuinely unclear to me which you prefer, why, and what signal you get beyond willingness to follow an instruction in a peculiar context.
We had candidates that were 'frustrated' because of the code. Some were frustrated that the code was low-quality, others that it used Canvas for drawing and they have never used canvas. Others about the way the logic was implemented originally.
The best candidates worked with what they had, and used the "tools" at their disposal: for example, use the code that draws a circle in the canvas already, dont try to reinvent it. At the end of the 3 hours, we talked about their experience doing the project and they had good feedback.
The bad candidates just deleted most of the code and added their own logic. My thought process is that, I could imagine them on their first day of in the team: the code startups have is never "the best one". I dont want someone who wants to throw the baby with the bathwater, but someone who can improve what we have.
A detail is that the challenge lasted 3 hours. Of those 3 hours , 1.5 hours wage candidate was "pair programming" with some developers (segments of 30 min spaced to give the candidate time to work alone). And during the pair programming , the devs were instructed to help him, give them hints and ask "revealing" questions.
These are good skills to have when a dev is debugging as they could get tied up for a long time dealing with something as simple as a relative path issue.
heh I like it. I'm going to put you as an intermediate skill level dev.
So these things break and need to be fixed. They also have to be setup by someone. You also need to know when something breaks, can I rule out that it's not a setup issue, a git merge that's broken a path, a user that's pushed a local dev config file accidentally.
In order to be effective, you need to have a feel and justification for the way things are. Getting someone to setup a project correctly should be a quick and easy way to do this.
For example, I've had candidates submit tech tests with code in the top level directory instead of having it in a src or some other sub folder. Which is obviously a red flag.
What you won't be testing is higher level skills. It may be easy to hack into a known codebase, and even do it cleanly, you just have to mimic the surrounding code. Getting the big idea and making long term choices is another set of skills, not exclusive with being a cracker, but not something you can see in 10 lines of code.
The higher level skills can be somewhat tested by asking the candidate how it would build some module and the variation that some factors would bring in (like closer deadline, no third party service,...)
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 what you are getting yourself in to. Also looking at the current open pull requests and closed pull requests to see the standard and speed of delivery. Bonus points for no PR’s and trunk driven development as that shows a very mature team.
My simpler interviews have often been with companies that have held a higher bar than the ones with tougher interviews. Those companies have often been sink or swim though and if you don’t make the grade you’ll be kicked out pretty quick. My last company had a reputation for new starts disappearing and not great that way, but the team was probably the strongest bunch of people I’ve ever worked with as only do good survived.
I'm not sure what Trunk Driven Development is, could you elaborate?
So trunk driven development means no PRs, until you decide you want to use PRs?
Short-lived branches like that site's describing just sounds like a team that's adopted git pretty well, but not formalized usage into gitflow or similar...
Feature flags can be used to gate features until they are ready, but all code is committed to trunk and is deployed to prod mostly continuously.
Bizarre.
There's good odds that it's the 26836th build, the 4045th snapshot, and the second time they've broken the file format. (or the 2nd expansion, or both)
Btw trunk based in general can still have short lived branches and prs. That’s what most public projects on github are doing
Pretty striking difference.
That being said, pull requests / code reviews / merge commits are totally compatible with that. Gerrit, for example, does every commit as a mini-pull-request on top of a branchless HEAD, and when the review finishes, you merge to main/trunk and off to CD it goes.
Bring in a spit and polish crew and build towards an evergreen codebase.
I agree, also sounds like an unusually well defined role with a clear way of having impact. I would also take this job any day over another job that would bait and switch me into some rewrite death march.
But yeah, when you think about -- a sufficiently fecal-encrusted, lost-cause codebase is almost indistinguishable from an actual greenfield opportunity.
Sounds perfectly sensible, and not naive at all.
I'd much rather come into a situation where they were both already aware of, and entirely honest about the current shit-show status of the project -- than a situation where everyone thinks what they're doing is great and bleeding edge but when you actually take a good look at their assets, as it were... not only are they manifestly and visibly unwashed -- but no one seems to notice the stink.
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.
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 your team.
The best teams I worked on who delivered working code fast, efficiently even when they are some of the most complex projects I worked on committed straight to trunk, had a very good build pipeline (super import) and worked closely together for review. The standards where extremely high yet the general feeling was it was less dogmatic, micromanaged or kept behind a gatekeeper.
The projects I’ve worked on with dogmatic pr’s generally failed to deliver anything in any amount of reasonable time. 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.
And pair programming? No thanks.
I think it you are doing actual code reviews, you are doing something wrong.
Smaller companies can pay much more (in expectation), and can simply poach the cream of the crop (who are invariably frustrated by useless process and corporate politics) from the big ones.
On top of that, most development processes are designed to minimize the blast radius of underperforming engineers, so your actual bar is "hire stronger engineers than the dregs from the giant shops".
Having engineers better than the competent engineers that are responsible for the dreaded process at the FAANGs, MS, etc is hard. The review policies of big projects tend to have been hard learned, and dismissing them because "they're useless process" is a poor choice.
I think you're conflating whether a process is useful for the organization with whether it is useful for the engineers implementing it. Since processes have to be uniform, you need to either figure out how to retain someone that's been at the company for a decade to continue to work maintenance jobs, or you need to figure out how to keep the fresh grads that will replace them from breaking the world.
The right choice for the organization is to keep production stable, and the bus factor high.
The right choice for the engineer is to demand massive comp increases (comparable to startup acquisition windfalls), and also the abilty to stop working maintenance.
A lot of the best engineers I've worked with used to be in a FAANG. Almost none of them would consider going back. They universally cite broken processes as the reason they left.
As one example, do you think cross-functional changes to the Linux kernel from even trusted contributors can just be merged without review and feedback from people across all affected subparts of the kernel, as long as they have been written through pair programming? That's a big open source example, but plenty of companies have plenty of projects for which that already applies as well.
> 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.
That is an easy trap to fall into (and wildly annoying), but it does not mean that PRs have no use outside of that.
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.
Fuck that dogpile of bullshit.
You've never seen a 1200 line diy function to parse XML have you?
:)
shudders
Yeah but the tool itself is antagonistic, because it imposes a workflow of open source, a workflow that also is antagonistic with clear and absolute power. So using that tool suddenly brings that antagonism and power into a team which is supposed to be 100% collaborative, and it also only does it periodically and with random and different people in power. It's not hard to see how that becomes somewhat complicated.
If a team inside a company want to gather inspiration from the open source world, they should look at how the owner teams of open source projects work internally, not how they work together with outside contributors.
And, it's a very common "solution" in tech to just simply not have any feelings, but people have feeling and that you can't turn off, and that's a very important contributor to work satisfaction.
The only thing PR's lack in that regard is that if the reviewer accepts it then it gets added automatically, while ideally you should re review things after the reviewer has accepted it. That way they wont accidentally accept prototype changes.
The tool does not stop you from working in the way you suggest. Maybe it's not what the engineers who wrote it originally envisioned, but it's both simple and easy to work in the way you suggest.
Yeah good luck with that, nobody can completely separate criticism of their work from criticism of themselves. You are making your job as the team leader way to easy for yourself, "I only hire robots, that's how I solve all these pesky people issues".
It's the bare minimum of professionalism.
If you are unable to separate feedback on your work, from attacks on your person, you are lacking some fairly fundamental skills needed for professional engineering.
Or for any job role at all?
Yeah I really hate those pesky automated linters running in standardised environments telling me I’ve screwed something up.
(/sarcasm, hopefully clearly!)
The guy who couldn't take feedback (person A) had code merged in that wasn't properly tested. Person B said, "hey A. I could use some help. We wrote some tests around $feature that were missing and the tests show the feature doesn't work. We see $unexpected results. Wanna make sure we agree on how this should work."
Person A: no, it works, I tested it.
B: could you help us identify the flaw in the tests then - they seem rock solid.
A: no, my code works.
B: ... ok, can you join us and talk through it?
A: no, it works.
A was removed from the team after management came in and A continued to not acknowledge his code could be wrong.
This was aberrational. We, as an org and as a team, constantly strove to keep the focus on the quality of the code. And, yes, his code was borked.
My suggestion is more along the lines of: use a pairing session for review so that you can bring your empathy as well as your technical expertise, and make it a step in the process just like any other steps (testing, PO approval) etc, and just trust people to do it.
I don't think there's any reason to use a tool from open source, to make code review enforced and with passive aggressive online communication and "blocking". Just seems to make work more painful, and less efficient as well actually.
And yes, someone that lost is pretty rare, but I'd say lower-grade versions of non-transparency and making their work hard to follow is pretty typical (and frustrating).
I've had patch review as back and forth comments in email, in bugzilla, in myriad other bug databases.
If you can't send out an email with your patch as an attachment, and get feedback, then we have a problem, and the problem is not the adversarial nature of review.
The goal in review is to try to catch any oversights or errors in the code you wrote. The code you wrote may have been the result of of discussions with your coworkers, but the code still gets reviewed, even if by people you discussed the implementation with. That review will occasionally find things you missed, and then you can cycle, and re-review.
By definition, the review can only happen at the end of change development.
Your very premise is wrong. At any sufficiently large company, you are unlikely to be sitting in the same room as every collaborator and stakeholder, or even proxies for them.
As a simple example, the team I currently work on (on one project of several) is 10 people across 8 US cities in all four US mainland timezones, and the stakeholders and collaborators are across Australia, Asia, Europe and the Americas. A good majority of what I do is pair programmed, yet the pull request workflow is essential to letting _others_ know what is happening and why, and to allow them to have asynchronous input into the process.
You might argue it would be better for the team to be in a room somewhere. Maybe so, but the people this project demands live where they live and could not even agree a common location for that room to be if they wanted to. And it still wouldn’t help the other projects…
Is code review not collaborative in your experience? In every team I've been on/run we've split code review comments between suggestions/questions and actual "I will not let this go into prod if my name is attached to it" blockers. Code review has been 99% the first set of comments and only rarely do I see anyone actually block reviews over things like style and what not that have been mentioned here.
I can't even imagine those topics getting into code review as a blocker as if we have actually strongly held opinions on mechanical issues like that, they are integrated into the various linters used so you don't even need human eyes to catch the issue.
Only if some regulation requires sign offs from e.g.other depts. PRs are inevitable. In all other situations they are at best an inneficient workflow and at worst a Kafkaesque circus.
Peer programming, daily checkups, a rock solid CI, and, above all, trust in the professionalism of your team are some ingredients for high quality, high throughput software development.
This is not an opinion. It's a scientifically proven fact. As laid out in the book Accellarate.
Absolutely agree, the collaboration should happen sooner in the process, and I would add that the team should probably also have made at least a high level proposed solution together before the work even started.
The point was not that "review" is bad, or should be avoided. But that a Pull-Request is a poor way to do that review. Reviewing is crucial, IMO.
I've found PR's become important approximately when feature momentum starts to drop.
Everyone is “gated” on a code review. The PR is one mechanism by which you can make code reviews easy. It says nothing about the strength of the 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??
Though the size and risk of a project do materially matter. If chrome ships a bug, that'll cause an impact on billions of people potentially, where as the typical software bug will impact a few hundred or dozens of people?
The way you get any project to be "actually good" is by adopting methods to ratchet quality. Early WebKit did not have the same strict rules it has today, and we suffered because of that. Introducing review + test for every change rule was introduced, and the frequency of regressions dropped dramatically.
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.
(Automating code quality stuff with linters is a no-brainer though.)
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.
As you write there is much more to it and one can only see through it after joining company.
For me trunk based development alone would be indicator that company is immature and does not even know they can have a process.
Like you totally disagree.
I described my experience and what I saw, well I did not do any scientific research on 1000s of companies. But I still think my experience has the same validity as your statement. So it can be both at the same time, there is so many companies small and big.
Some might just push commits without knowing or caring about reviewing code, beyond fixing what fails in production. But others might just do "git flow" or whatever, doing thorough PRs, without knowing that the changes could be requested after being merged and without realizing the amount of time that is wasted on integrating code and re-testing.
In particular, I think that PRs come from an open source model, where you really have to gatekeep. But in a company, there is no problem in following up: the devs are salaried and you just need to put them to work on whatever is necessary.
I also don't like PR's/Unit Tests/other practices motivated in a way: "because that is what professionals/google/microsoft does".
Thinking it always does or doesn't make sense just tells me you don't have experience working on diverse projects.
Yeah exactly, PR's are based on the fact that you have some person who is the owner that have complete power, and many other contributors who have zero power and whose contributions will mostly be rejected. You simply don't have that situation in a company, where everyone is an owner on equal terms, and all contributions are accepted, only some with slight modifications.
So you get these really weird situations where more junior, or less skilled, people can block PR's and demand changes from other more skilled and/or senior people. And if there's disagreement in a review: the resolution of that is completely unknown, and can often blow up to a really nasty conflicts.
I have so many terrible experiences with PR's where you get hostile nonconstructive comments on your work, on github, from a person that literally sits next to you. It's the creepiest thing ever.
I'm getting downvoted because I'm criticising developers favorite tools that lets them pretend to be Linus Thorvalds for a moment.
PRs can be approved based on two people's opinion. There doesn't need to be a central gatekeeper.
> So you get these really weird situations where more junior, or less skilled, people can block PR's and demand changes from other more skilled and/or senior people.
Sometimes junior, or less skilled people, have something valuable to say. Especially if the code could be simpler.
In a stalemate, the PR could be sent to a third party. I've suggested this many times to avoid unnecessary conflict.
I don't think it is PRs that are the issue, rather your working environment.
Yeah and sometimes they are naive, dogmatic and overconfident, and on a crusade to change all the things! because they have read some blog post by uncle bob, and this tool is putting them in absolute power every time they do a review.
> In a stalemate, the PR could be sent to a third party. I've suggested this many times to avoid unnecessary conflict.
Ok and who might this lucky scapegoat be? I have a feeling it's not the manager for some reason..
> I don't think it is PRs that are the issue, rather your working environment.
The issue, which I'm trying to illustrate, is that the tool makes the working environment worse by introducing (hostile) dynamics between people that don't exist, which leads you into situations that you don't have resolutions for, situations that should not occur.
Using a tool that allows you to block other people's work causes unnecessary conflict in a team where people are supposed to be working together.
Edit: blocking contributions is a normal and natural thing in an open source workflow, and it is not normal and natural in a team inside a company.
But I'm okay with the conflict! There should be conflict at work! Ideas should be freely expressed and those ideas are going to meet contrary ones!
What wouldn't be healthy is a place where that conflict isn't resolved or doesn't lead to a better idea winning. Or where only the Lead "wins" because of their position.
There shouldn't be arguments, no one should yell or be hurt. For sure that's a bad place to work. But conflict about where to place a piece of code? Sure! Conflict about if we name it Foo or Bar? Why not?! That conflict is like the sharpening of Iron! It hurts _today_ but can strengthen and make _you_ better let alone the organization as a whole.
I dearly hope this is sarcasm. This is about the same level of absurdity as developers taking ages to pick project/file names. I'm not railing against a review's abilities to find bugs and make sure someone else understands. GP is right in pointing out how many fruitless review discussions exist over personal differences in what to call a function name because "I think X sounds better than Y", despite every party involved understanding the code and what it does.
If only linters could solve these trivialities.
IMO every developer should be an admin of the repo, but more importantly it’s not a gate, it’s a process that benefits communication. All the team can read and learn from small addings to the code base.
PR reviews should not equate to 100 comments either, complex discussions can (should imo) be worked out in a call discussing (dialog) best approach. Think of white boarding a problem with a fellow engineer friend.
Ofc, teams differ and some teams work better with other processes and other tools.
It's the tool itself and it's imposed workflow of blocking work and gaining absolute power in demanding changes, that causes the working environment to become toxic. Nobody would ever do that in a meeting "I'm blocking this work now until my demands have been met". That would be incredibly hostile, but with this tool it becomes normal.
PRs are a great way for the whole team to learn about how the organisation cuts code, and can reduce the number of errors, but of course with poor leadership they can be used for evil.
I can imagine that you had fun mingling with the commoners for a day, now try it on all of your actual work, and with the whole C-level team gang up on you for each review.
> about how the organisation cuts code
You don't have any guidelines for how the organisation cuts code, and that's why you like the review process, because it covers up for that.
> I can imagine that you had fun mingling with the commoners for a day, now try it on all of your actual work, and with the whole C-level team gang up on you for each review.
You literally know nothing about me, my experiences or how I conduct my life, and rather than listen to people with a different experience to yours, you choose borderline abuse.
This is the comments section of HN. Take a break and get some perspective.
Maybe your work experience is mostly in a high stress, prototype-heavy environment where it's more important to launch something than it is to have a maintainable, incrementally improving codebase? I worked at a consultancy like that and it was very different from "big product" long term work.
As for the social dynamics, it sounds like your workplace culture just blows.
"Code reviews", in general, are a good thing for knowledge transfer. If they are done for nit picking and stylistic complaints, they are not terribly valuable.
Because in my experience, reviews and PRs certainly Ven damage latency, but overall throughput remains the same as it would be with subsequent follow-up fixes of these issues to the trunk.
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.
The PR and original branch show dead-ends that should not be required reading to understand things.
That way, I can treat the series of merge commits to trunk as the simple, linear overview of history, but when I'm bug-hunting I get small, clear commits to search through with git bisect.
It also means I get more useful blame output, as instead of huge hunks of a file being all from one giant bomb commit, I can see the process and evolution that the whole change went through as the original author pieced it together. That can be really helpful when dealing with obscure bugs or understanding systems with little documentation, by helping you get back more of the original author's thought process and seeing how their mental model evolved as they built the feature.
The "right" answer is probably to refactor the test suite to be more performant, but that's never going to get approved given the amount of work that would take, and it would take me longer than I plan on being at the company to get it fixed in my spare time.
I do have it passing all tests before I try to merge if that counts?
• All commits SHOULD pass all CI tests
• Merge commits MUST pass all CI tests
The reason I don't require every commit to pass all tests is to maximize reviewability and logical consistency of commits.
For example, if a file needs to move, and then be modified slightly to function in its new location, I prefer to break that into two commits:
1. A verbatim move operation. This commit will fail CI but can be effortlessly reviewed.
2. Small modifications. This commit will pass CI, and its reviewability is proportional to its size.
In contrast, if you smush those two commits together, it can be hard to see where the meaningful changes occur in one giant diff. (Some tools may be smarter about showing a minimal diff for a move-and-modify commit under limited circumstances, but that doesn't always work.)
All of these constraints can be enforced programmatically, and if you're going to adopt them at all I think automating them is the way to do it.
Personally, whether I enforce this branching strategy varies from team to team and project to project.
Many projects I've been on had much, much bigger issues to deal with, so something second-order like this never gets to the top of the stack.
That said, it's an approach I like, and I think it yields benefits if you have a team that's bought into it.
For example:
When I want to see tests or documentation or config related to a change, I'll find the commit and look for other lines changed at the same time.
When I make automated changes to the code, like reformats, auto-corrections by a linter, or IDE refactors, I create a separate commit to separate mechanical changes from those that require more human scrutiny.
In some ways it is unfortunate that services like GitHub and GitLab have become so dominant in the industry. If you're just working with plain git there is no assumption that squashing is some kind of binary decision the way the UIs of the online VCS services tend to present it. It's normal to do an interactive rebase and squash some commits to clean things up before sharing your code, yet keep logically separate changes in their own distinct commits, and you can have a much nicer commit history if you do than with either the no-squash or squash-everything extremes. Of course you can still do that with something like GitHub or GitLab as well but I think perhaps a lot of less experienced developers have never learned how or even why they might want to.
Creating a separate branch, pushing this to your public copy of the repository and then asking someone to pull from that branch into their master branch seems absurd to me, especially if it's just 1-2 commits, and the idea of reviewing code (which I think is extremely important at team scale) should not be conflated with the concept of pull requests.
People look at the PR (which shows the diff) and approve it if they are happy, or request changes. If there are no conflicts, you can merge with a single button. Otherwise you need to resolve conflicts somehow. I normally just rebase the branch off updated master and force push.
> Creating a separate branch, pushing this to your public copy of the repository and then asking someone to pull from that branch into their master branch seems absurd to me.
That's how it works for open source projects because people do not have permissions to make branches in the main repo, so they must fork and have the changes in a different repo. I've never seen this done at a company, I presume everyone here is talking about creating branches in the main repo and requesting review before merging the branch to master.
Regardless of whether someone is pushing to a branch in the upstream repo or pushing to a branch in a fork, the workflow is the same either way. At worst, it just means adding a remote if you want to check out someone's code locally.
Either way, if you read the thread, the folks are arguing that giving someone the power to block code getting into the mainline is bad. Judging by what you described, where someone has to approve before it can be merged, we both agree to disagree with them. We are just arguing the semantics of how that is implemented.
Either way, just so you know, this kind of attitude and that you think it's a-ok to use such a convoluted process for what is effectively the same thing would 100% mean me not hiring you. That may not mean much on some random forum on the net, but you will encounter it a lot as the industry is definitely not aligned to this odd flow you described.
Now they're immediately available for review. That looks something like this (this is not from my workplace, but the Go team's Gerrit instance): https://go-review.googlesource.com/c/go/+/361916
> Either way, just so you know, this kind of attitude and that you think it's a-ok to use such a convoluted process for what is effectively the same thing would 100% mean me not hiring you.
That's fine. There are plenty of companies that manage to hire based on competence, experience and references so I'm not exactly aching to get hired at a place that would deny me for having used Gerrit.
Maybe the kernel of a good idea in the comment: A team that prioritizes reviews, such that PR's are not a significant hurdle, is indeed a sign of a mature team.
Good candidate would instantly notice the team is being labeled as seniors but instead are a bunch of crappy devs with years of experience.
Code does not lie. If you are crap you gonna produce crap code.
Ive recently hit such a mine. Lies during interview. Gonna sit here through vacations and jump the ship. Was sold on working with experienced ppl with over 10y of experience. Turns out those ppl IMHO have less skill than me few years ago with only 2y experience. Legacy bugers that did not improve over the years.
If you're on a team that isn't currently having staffing issues, I don't think it's weird to want your next hire to be better than the median person on the team. "Raise the bar," etc. Asking harder leetcode questions seems like a bad way to achieve it, but I get the impulse. If the bar for member nine of the team is approximately "better than the bottom 4" then that 9th person in is likely to have at least one moment of "wait, how did you pass that interview?" at some point after they come on.
If you say you know tech X, manager wants to hire for tech X to solve a business need, and I'm asked to help interview, I may ask you questions about things I don't know or do myself because it's my professional responsibility to my employer and manager to help them hire a quality candidate. Maybe some people use the interview as a place to be jerks or show off but that's not always true. Personally I'd rather not be involved in the process at all.
Riddles I've encountered while working as an engineer: 0
Happens to me too, I get tough interviews, and a good salary, but the work is exactly the same as the juniors "a guy on the scrum team". I don't even understand why they hire seniors, or have that title when it means nothing. I guess they expect them to be just faster versions of juniors.
No, they expect them to be:
- more independent
- able to bring past experience to bear on present problems
- make fewer mistakes
- be able to help others rise to the next level
- possibly make good team leads at a later stage
I spent most of my time writing design documents, doing code reviews, and shepherding my relatively small changes through review. Something I could've banged out in a week would take months due to all the review and processes.
I hope they weren't some gatekeepers you ran into... "don't use emacs and fish and the dvorak keyboard? you don't fit in here."
Trunk driven does not mean mature team 100% of the time, but if you have a mature team trunk-driven is more efficient than PRs.
It only works if either everyone is senior, or it’s a project of 1-2 devs, or if people are pairing most of the time.
This is not what I would call a "strong" team. This is what I would call a developmentally stunted team. By that I mean they've reached competency as individuals, but they have limited ability to level up fellow developers. As such, they are not a producer of talent and rely on actual strong teams for their hiring pool. In my experience, these teams are best avoided if you actually care about growth.
Firstly, you can work with realistic code. It can use your real tech stack and there can be enough code with a realistic structure to see how a candidate finds their way around.
Secondly, you can make it open-ended. For junior candidates it might be best to stick to simple questions like what does this short function print and see if they can reason through some basic logic. But for more experienced candidates it can be a general code review.
Your example code can include anything from superficial mistakes like typos and unnecessary duplication to strategic problems like inflexible designs or inconsistent models. You can include obvious or subtle logic errors and performance problems appropriate to the level of the role.
Ask each candidate to talk you through what they're thinking about as they read the code and see what level they work on and how much they find in the time available. Are they methodical? Do they flag the trivial stuff but not get hung up on it? Do they spot the O(n^2) algorithm? Do they spot that algorithm but then start talking about worst case vs amortized performance and using a profiler to decide whether changing to a theoretically better but much more complicated algorithm would be justified?
In this kind of environment you can quickly tell if you have an awful or outstanding candidate. For the majority who fall in between you have an objective basis for comparing their performance. And all without any trick questions or LeetCode junk at all.
As I'm a pure C coder, my starter question was usually something like:
a). What does malloc(0) return ? b). Why does it do that ?
I'm kind of at a loss about why it can return either of those two things, somebody want to take a shot at explaining it?
Answers to this question taught me about the candidates taste and understanding of good API design :-).
Both NULL and "unique pointer" are correct answers, but all modern implementations only chose to return one of these. My follow-up question is "why ?" :-).
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 realloc() which may favor NULL or a non-null pointer to 0 bytes, but that's too much work to figure out now.
Suppose that malloc(0) returns a not-NULL value. Is malloc(0) == malloc(0) guaranteed to be false? (I think that it is, which is how ENOMEM can happen.)
So, the "right" answer is probably malloc(0) returns a unique pointer because then the error check is simpler - a NULL return value is always a true error, you don't have to look at size.
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 zero size passed in ?
glibc and all implementations distinguish the two by allocating a internal malloc heap header, but with internal size bookkeeping size of zero, returning a pointer to the byte after the internal malloc heap header.
The only valid things you can do with the returned pointer is test it for != NULL, or pass it to realloc() or free(). You can never dereference it.
Returning a valid pointer to NO DATA is what all modern implementations do when a size==0 is requested.
In an interview situation, discussions around all these points are very productive, telling me how the candidate thinks about API design and how to fix a bad old one, whether they know anything about malloc internals (which is essential as overwriting internal malloc header info is a common security attack), and how they deal with errors returned from code.
Remember, it was only my warmup question :-). Things get really interesting after that :-) :-).
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.
malloc(0) != malloc(0) because the internal malloc header is different for each allocation.
Asking for a malloc of size n, means internally the malloc library allocates n + h, where h is the internal malloc header size. So there is always an allocation being done for at least a size of h, just with an internal bookkeeping "size" field set to zero.
and yes, while(1) malloc(0); will eventually run out of memory, counter-intuitively :-).
>
> I'm kind of at a loss about why it can return either of those two things, somebody want to take a shot at explaining it?
Any return from malloc, whether it succeeds or not, is a valid argument to `free()`. Hence, it can return NULL because `free(NULL)` is legal, and anything other than NULL has to be a unique pointer, because if it returns a duplicate then calling `free()` no the duplicate will crash.
FYI. I introduced a (bad) security bug into Samba a long time ago by expecting the wrong return (i.e. the one the implementations never return :-).
I once flunked a faang interview because the interviewer mispronounced (or used the correct pronunciation I was unfamiliar with) of "arp protocol"
I had no idea of what was being asked and was racking my brain for something I didn't think I had ever used to down every computer in the library of my high school 12 years before.
Imagine you just read a 4 byte value off the network, and it's part of a protocol that specifies how many more bytes there are to read. You might (in error, ahem... :-) pass that value to malloc(). So knowing what might happen if an attacker puts an unexpected value in there is something you need to think about.
If you don't know or can't guess because you don't know how malloc() works, then you're not the person I'm looking for.
In the end this is still a language lawyer question. It's a technicality that should never be relevant. If it is you've already gone wrong several times. In other languages there might be an argument that it probably does something reasonable and any developer experienced with that language should be able to make an educated guess about what that would be even if they don't know. But you asked about C, a language infamous for having undefined behaviour in many such situations, so I don't think even that is a particularly compelling argument here.
Sure. So do many other protocols and file formats. But if you're using those values for memory allocation without checking them first then getting a 0 might be the least of your worries. Unchecked large values might be a DoS attack waiting to happen.
If you work with C code where security is a factor then surely you already know this so it still seems odd to put so much emphasis on your original question. You do you I guess. :-)
Source: worked at Google.
Yea. It would be that way.
Secondly, why should a C application developer know enough about the implementation details of malloc() to answer such an esoteric question? Malloc can not, by definition, be implemented in C, so it seems a bit out of scope.
Of course in reality there's probably a few "non-standard"[2] system calls in the path, mostly mmap.
[1] Not necessarily, you can place them at special places in your my_memory array by convention, e.g. at the start of the array.
[2] I assume that we're excluding standards on other layers like POSIX, otherwise it's trivial.
http://jemalloc.net/jemalloc.3.html
"Traditionally, allocators have used sbrk(2) to obtain memory, which is suboptimal for several reasons, including race conditions, increased fragmentation, and artificial limitations on maximum usable memory. If sbrk(2) is supported by the operating system, this allocator uses both mmap(2) and sbrk(2), in that order of preference; otherwise only mmap(2) is used."
Also, Google's tcmalloc uses mmap to get system memory:
https://github.com/google/tcmalloc/blob/master/docs/design.m...
"An allocation for k pages is satisfied by looking in the kth free list. If that free list is empty, we look in the next free list, and so forth. Eventually, we look in the last free list if necessary. If that fails, we fetch memory from the system mmap."
In fact I'd be surprised to see a modern malloc implementation that doesn't use mmap under the hood.
For example, I hired a backend developer last month. She already had the job because she came highly recommended from a trusted friend of mine, but she didn't know that. Here's how the interview went down:
Me: I see on your resume that you've achieved Grand Master level in Microsoft Solitaire Collection.
Her: Yes.
Me: Well, we won't waste any more time then. Welcome to the team.
This is my first time experiencing HN deja vu :)
I still give my team the option to decline them if there's major red flags - but I have not had that happen.
Also, a reminder...an interview it is a 2 way street. Your 1 question interview doesn't give the candidate an opportunity to interview you or your team and the "Welcome to the team" is a little presumptuous...I assume it was left out for dramatic effect and I'm sure (I hope?) you allowed the interviewee time to ask questions.
It was even more impressive because the person doing the interview wasn't super competent and yet they still managed to do a fairly comprehensive technical evaluation in ~2 hours.
One section was a debugging session where you had to get tests to pass. The code and tests were quite decent, which made it quite easy to show off. Every company should do this.
Too bad the Triplebyte promise (not having to do phone screens) was a joke and their business model wasn't good, but that part of Triplebyte had major promise.
This is true. Most of the time I'm only reading my own code, and over the years I've been motivated to code better by having to go through crap I wrote early on.
Are they intentionally trying to scare devs away?! (Some of the code bases that I've seen have been pretty bad)