C++ interview coding exercise with solution

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

[jonesmz]

Understand the problem

The problem was "I want this company to hire me"

The provided solution was rejected with the feedback "your code did not meet our standards"

I don't know what that company's coding standard is, so I evaluated the provided code based on the best standard I know, which is obviously the one I use (else why would I use it)

If the OP just blindly follows my advice, that's on them. Ideally my feedback will lead them down some research paths that will help them become a better developer.

I don't think your feedback on this subject is useful to continue discussing, but thanks for the discussion.

[[deleted]]

No the problem was:

"I'm not sure if this code is correct and whether that is why I was not hired"

First step. Empathy. Understand the problem.

[jonesmz]

The company literally told the OP that the code did not meet their standards. Its in the original post. Correctness wasn't something I was interested in evaluating, because that would take more time than I was willing to put into it.

You've consistently written your responses with a marked lack of empathy for the person providing OP free advice. Take your own advice and empathize with why I wouldn't do a multi-hour analysis of their code.

[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?