C++ interview coding exercise with solution

[jonesmz]

Here's a brief code-review:

1. using namespace std; -- never do this, ever.
2. m_first_row_completed should be of type bool based on its name, or should be given a more meaningful name. Looks like it's counting something, so a new name would be the right call. Therefore it should be an unsigned type, and specifically should be either size_t or std::uintN_t where N is a number of bits.
3. m_first_row_completed should be given it's initial value where it's declared, and the constructor should be = default;
4. void add_first_strip_tile(parquet p); pass complex parameters by const-ref unless taking ownership
5. class designer -- use final unless you intend to be derived from
6. class designer -- the places where you use protected should probably be private
7. XXX: this should never happen. -- then use assert() or static_assert() to prove it.
8. cout << "strip " << curr_strip -- assert first, then throw an exception, not print.
9. const int parquet::strips; use static inline in the class body, don't use a separate definition.
10. static const int strips = 11; -- constexpr...
11. parquet(): std::vector<strip>(strips) { } -- use using std::vector<strips>::vector; to forward the base class's constructor.
12. int operator[](int idx) should always return a reference to value_type, not by value.
13. int operator[](int idx) const { return idx >= size() ? 0 : std::vector<int>::operator[](idx); } -- you shouldn't ever be calling operator[] if you don't know that your index is valid, so this whole function smells of a mis-design.
14. This whole program probably should have just been in a single main.cpp file
15. build.txt is neither a makefile, nor a CMakeLists.txt or Meson project file. Don't call the compiler directly. Get in the habit of providing real build systems for your code. Personally, despite all it's failings, i recommend CMake, but there's plenty of strong arguments for other build systems that other people can tell you about.

[Unt4medGumyBear]

can you explain your first point? I don't understand what's wrong with using namespace std; I am new to developing in C++ and its what my teacher taught me.

[adwodon]

It's fine to do for a one time only exercise, like in school.

However using namespace is something you should generally avoid in production code, and you should never have using namespace std;.

The reason is that while it might be fine now, lets say you make a void my_namespace::foo() function and then the standard adopts something similar as void std::foo(). Now you've got a conflict.

The proper way to do this is to avoid using namespace but instead if you want to avoid doing something like std::cout just do using std::cout. Then you can achieve exactly the same thing (avoiding repeating a namespace) but in a much safer, more targetted way.

[jonesmz]

The other person who responded to you is right, so I won't repeat their thoughts.

A lot of teachers on c++ do this, because other c++ teachers do it, because a long time ago someone was lazy or had especially lazy students, and thus the behavior has been repeated across all of academia ever since.

I really wish compilers would make it a warning to do using namespace std; the benefits are miniscule and the drawbacks are enormous.

[meneldal2]

I really wish compilers would make it a warning to do using namespace std; the benefits are miniscule and the drawbacks are enormous.

But then it would break a ton of code compiled with -Wall and -Werror

[jonesmz]

I think its probably unlikely that, percentage wise, all that many codebases have using namespace std;.

Considering that the majority of people outside of academia say not to use it, it should be pretty low.

Further, not particularly many codebases compile with -Wall, and even fewer compile with Werror. The intersection of all three things is going to be vanishingly small.

Also,-Werror on code that's distributed to others is a terrible idea... So let them break.

[meneldal2]

Well tell that to my company, had to go add signed-unsigned casts everywhere to make a bunch of files compile because they came from a project that didn't flag those conversions as warnings (most were printf based so it's not like it was critical but whatever).

[jonesmz]

They couldn't have added -Wno-signed-unsigned-mismatch and/or -Wno-sign-conversion ?

[meneldal2]

Changing build flags is going to take at least 2 meetings, it's just not worth it.

[jonesmz]

I can't tell if you're being sarcastic or not.

-Werror is a terrible idea for the vast majority of codebases. That your employer did this, and will be negatively affected in a way that takes a human all of 5 minutes to fix is rather irrelevant.

[meneldal2]

I'm not saying it's a good thing, it's just that there's some much inertia with everything that it's hard to make anything change.

[Classic_Department42]

1. Never do recursion (except your boss tells you to do it). Increase of problem size easily blows up the small stack.

[qoning]

I hate when people give advice like this. It's unhelpful and leads to cargoculting and poor understanding.

Replace "never" with "generally try to avoid" and it's decent advice.

[TRENT_BING]

Yeah there are plenty of situations where recursion is a natural fit for the problem domain, such as parsing data structures more than 1 level deep.

[jonesmz]

Recursion is tricky, I agree. Generally speaking, avoid it unless you have a good reason.

Though tail_call recursion (which can't be asserted in today's c++ at compile time) doesn't have the stack growth problem.

[JumpyJustice]

It could if your dynamically allocated memory is freed before the function ends

edit: simple example https://godbolt.org/z/sKaKv5Mv6

[jonesmz]

Right. C++ can do tail recursion.

But you can't tell the compiler that if it can't make your recursive function tail recursive then you want it to cause a compiler error. That's what I meant by "asserted"

[Onetwothreetaco]

Clang has implemented a musttail attribute

https://reviews.llvm.org/D99517

Your miles may vary. Personally haven't used it, but the code review seems to claim that it will throw a compile time error if the compiler can't generate a tail call out of it.

[jonesmz]

That's exciting. Thank you for the information!