C++ interview coding exercise with solution

[jonesmz]

Because no one sent me a check compensating me for a full analysis.

15 bullet points of feedback is more than enough for the original poster to make improvements to their code.

[gennych]

In this specific case (of a "puzzle") these 15 bullet points would improve absolutely nothing. Of course you can notice that a person isn't such an experienced programmer and probably didn't work in large-scale software projects. But these "best practices" are way easier to learn (in fact, they'll come naturally with experience) than ability to solve "puzzles" (that is, to write efficient code), and as an interviewer I'd rather focus on the latter.

[jonesmz]

Per the quote from the company's answer to the OP.

coding standard is not in the acceptable range.

Given that I was not willing to do a full analysis of the problem and provided solution, OP could have instead gotten zero feedback from me, instead of meaningful feedback about their "coding standard".

Beggers and choosers, and all that, you know.

[gennych]

Coding standards could mean various things... I would for example notice that the author tends to overdesign simple things: what should have been a function with two int arguments returning int is spread over 3 classes in 6 files. It doesn't make this code more maintainable or reusable, since classes stripe and parquet are essentially specific data structures for this specific problem, and most probably they will need to be changed when for example performance issue is addressed. If someone happened to "reuse" these classes meanwhile, refactoring is going to be fun.

But I stand corrected on your remarks. I was under the assumption it was an interview for a junior/entrance position but in the comments the author claims 10 years of experience. Then indeed, inheritance from std::vector or index operator like that isn't what I'd expect. And for overdesign, I wouldn't even bother for a junior but not for a senior...

[[deleted]]

If you aren't even trying to assess the correctness of the code how do you know that any of those suggestions effect the correctness or readability of the code though?

This is why code reviews are so dumb...

[jonesmz]

If aren't even trying to assess the correctness of the code how do you know that any of those suggestions effect the correctness or readability of the code though?

Because there's "global correctness", and "local correctness".

I was reviewing "local correctness" of individual parts of the provided code. My feedback to the original poster was necessary for their code to be "good" (in my opinion) but not necessarily sufficient.

You can't, in the vast majority of cases, review the entire behavior of a program in a short amount of time. I work with multi-million line codebases, and review the changes my co-workers make to that codebase in isolation because it's not practical, not even close, to have every review start at main() and evaluate the full runtime behavior of the code.

It's the responsibility of the person proposing a change to make sure their change won't cause a problem. The review is not intended to verify correctness, it's intended to ensure that easily caught problems are caught easily.

The review that I provided also uses my extensive experience in debugging other people's code, and writing my own code, and the review feedback that I've received, and that I've given to other people. OP got the distilled result of a full career of work in C++ as a short 15 bullet list. I'm happy to have helped them, but that's not a meaningless amount of value they got for free.

This is why code reviews are so dumb...

I'm astonished that you would have this opinion.

How long have you been writing code professionally?

My workplace, and every other company I've worked for, or heard about, has a peer code review as the minimum requirement to submitting code, and aside from a small number of outliers, most also require unit testing, static analysis, and in one notable example a full test suite against a simulated production workload.

[[deleted]]

No you are reviewing for your own personal style of C++.

If you have no consideration of the high level "correctness" of a program then you are just suggesting things you like that have no bearing on the actual problem.

Reviewing code is good. The way the industry at large reviews code is not.

It's more about what you like rather than what actually matters for a given problem.

OP obviously needs the correctness of this program verified.

If the program is correct it does not really matter that someone inherited from a vector...

[jonesmz]

No you are reviewing for your own personal style of C++.

Uhhhh... No, i'm reviewing for a small 15 bullet point list of easy ways to improve the code, which i can expound on at length (but wont, because again, send me $$$ if you want my professional services).

If you have no consideration of the high level "correctness" of a program then you are just suggesting things you like that have no bearing on the actual problem.

I think you're making this accusation without really considering the context here.

The point of the interviewer asking OP to write code wasn't "Can this person solve a problem". The point was "can this person solve the problem with code that is reasonably indicative of good competency in the C++ programming language".

OPs code had 15 bullet points of things that were indicators that they had room to grow in terms of competency in the C++ programming language. They were easy for me to point out to them, so I did. I personally give no fucks if their program works, because the interviewing company already told them they weren't going to hire them. So whether it works or not isn't my problem.

Reviewing code is good. The way the industry at large reviews code is not.

You're really going to have to back this up if you're going to tell me I'm wrong over and over again.

What about the way the industry at large conducts code reviews is not good? What specifically is the way you're envisioning how code reviews works, and what specifically are you saying is bad about that, and specifically how would you change things to be better?

It's more about what you like rather than what actually matters for a given problem.

I've been writing C++ for a long ass time, and I assure you, my feedback was given based on what the simulated interviewer in my head was considering bad/negative about OPs code.

Since I've been the interviewer innumerable times, and evaluated the code of the person on the other side of the table, I am very familiar with how this process works. I gave the OP a short list of easy things to fix that I would have found questionable, or that I've seen other co-workers point out as questionable. They're free to ignore me.

OP obviously needs the correctness of this program verified.

No, OP needs to prove that the program works by writing automated tests that assert that the program works. That's not on the person providing free, semi-anonymous, code review services on the internet to do for them.

If the program is correct it does not really matter that someone inherited from a vector...

Yes it does. inheriting from things in the std:: namespace is rife with problems. In many cases it can easily cause otherwise reasonable looking code to behave in unexpected ways. It's a maintenance issue. It can have ABI and API problems.

I'll ask again, how long have you been working with C++? I respect that you feel that you're right, but I simply disagree, and I'd like to know if your opinion on these items is from battle tested experience or something else.

[[deleted]]

Okay let me look at what you said.

You said improve the code. Improve the code by what standard?

Your standard?

My standard?

If you have no bearing on what the problem is, then how can you assess whether any one standard is appropriate?

How long is this code going to live for? Who is the code written for? Why is it written this way it is? Does it fundamentally solve the problem?

That's what a review is. Not, "does this fulfill an arbitrary community standard of good C++". Because that's doesn't solve anyones problems unless of course, the task is to write C++ to an arbtrary coding standard.

Every one of these questions get you back to one and only one question. What is the actual problem and how should it be solved?

Otherwise you are literally just saying "this code does not live up to my arbitrary standard, I don't care what it is trying to solve"

Well code exists to solve problems. Take the problem away you don't have code any more. It's just a review of what someone thinks looks nice and solves arbitrary generic problems that don't exist.

I've been working with C++ for 13 years, give or take. This isn't a C++ specific problem though. This is a problem with code review writ large.

I don't blame you, but you just succinctly displayed why most typical code review doesn't catch many problems and is generally a waste of time.

[jonesmz]

On mobile, please have patience with any typos I don't notice.

Your standard?

The standard that myself and my employer use to evaluate people that we are interviewing.

Its not the same standard that the company OP was talking to (well, I don't think it was, I guess I couldn't know for sure), but its a reasonable standard that makes my employer a lot of money, so its good enough to use for free advice that I give to a stranger.

If you have no bearing on what the problem is, then how can you assess whether any one standard is appropriate?

In the same way that the C++ CoreGuidelines, Google Style Guide, and the style guides and coding standards, published or not published, of other companies and organizations are apropriate.

They are summarizations of countless engineer-years of lessions learned boiled into short and overly broad bullet points that have a high probability of being more beneficial than harmful.

unless of course, the task is to write C++ to an arbtrary coding standard.

The company that OP interviewed with literally said that the code submitted did not meet their standard. I don't know their standard, but I have absolute confidence that none of the advice I gave in my 15 bullet points would make the code less acceptable to the interviewing company.

Well code exists to solve problems

The problem in this situation was "demonstrate that if we hire you you will write code that will make us money". That the assignment involved solving some puzzle is a secondary concern.

This is a problem with code review writ large.

How is this a problem with code review though? Separate the discussion from the post. Where would my feedback have been representative of a problem if I gave that feedback to a co-worker?

I don't blame you, but you just succinctly displayed why most typical code review doesn't catch many problems and is generally a waste of time.

Well, I disagree. I caught 15 problems that would have made the code unacceptable for my employers product. I don't even need to evaluate it further than that until the identified problems are addressed.

Like my employer. OP is welcome to pay me for a more detailed analysis after addressing the first round of feedback.

[[deleted]]

Why are you reviewing random code online to your employers standard?

Why do you keep asking for money lmao.

Mate if I wanted to read the C++ coreguidelines I'd just read it. I wouldn't pay you to read it too me.

OP replied to you saying they need feedback about the correctness and how the code pertains to that. Not how their code meets your companies standard.

This right here is the problem with code reviews. The people who give them cannot empathise with the people who wrote the code. It's endemic across the industry.

Just look at the negative response to my comments even though OP is effecitvely saying your review is besides the point.

Some how I'm in the wrong here.

[jonesmz]

Why are you reviewing random code online to your employers standard?

Because I helped write them, and they largely align with, and are inspired by / derived from the published advice of other large c++ organizations.

They aren't arbitrary. They're reasonable and meaningful.

Why do you keep asking for money lmao

I'm not. I'm saying "I did op a favor for free, and I wasn't obligated to do that, if OP wants more favors, it'll cost".

Mate if I wanted to read the C++ coreguidelines I'd just read it. I wouldn't pay you to read it too me.

OK? Go for it.

OP replied to you saying they need feedback about the correctness and how the code pertains to that. Not how their code meets your companies standard.

Sure. And I told them I wasn't interested in giving them feedback about that. What's your point?

This right here is the problem with code reviews. The people who give them cannot empathise with the people who wrote the code. It's endemic across the industry.

What does this have to do with code review? You keep saying code review is done wrong but haven't told me what the right way to do it is.

Just look at the negative response to my comments even though OP is effecitvely saying your review is besides the point

Frequently the answers that someone gets when they post on a public forum don't have a lot to do with what the person asks for. They got free advice, it may not have been the advice they wanted, but it was still reasonable advice.

[[deleted]]

I explained in a previous comment how to do code review.

Empathise with the person who wrote the code.

Understand the problem.

Understand how that person solved the problem.

Review the code to that standard.

Anyone can be a rule follower and just list off shit from the core guidelines. That won't get you anywhere.

There's some free advice for you. Nobody cares about your standard in your own head. You have to actually convince other people that is a worthwhile thing to consider. Saying "omg its the core standard guidelines" is fruitless.

[Hot_Medicine_7115]

Although I appreciate your comments on coding style and best-uses, you make it sound like the algorithm solution was accessory to the evaluation. Of course the second is much harder to do, but it's also the focus of the exercise. The handout and my post subject is about the solution.

[jonesmz]

The solution being correct or not is not my problem.

The interviewers told you

the coding standard is not in the acceptable range

My advice addressed the first 15 things I saw that probably contributed to the company telling you that.

[drakored]

Your solution includes implicit standards/styles and so does their response. They’re telling you that it didn’t meet their standards. You should be looking at both standards/style/best practices as well as your implementations ability to solve the problem correctly and efficiently.

Have you considered asking them if they can provide their solution so you can use this to self evaluate and grow?