Job Interview, Did I Mess Up?
in [C++]
|
From: Karl-Czoh on 3 Mar 2010 17:24 > // Answer 2 > // I told the interviewer that I wasn't sure what he meant by "generate" > // so I just sent the result to standard output. > void permutations(const char* in) > { > vector<string> strs; > stringstream ss(in); > string next; > while (ss >> next) { > strs.push_back(next); > } > copy(strs.begin(), strs.end(), ostream_iterator<string>(cout, " ")); > cout << '\n'; > while (next_permutation(strs.begin(), strs.end())) { > copy(strs.begin(), strs.end(), > ostream_iterator<string>(cout, " ")); > cout << '\n'; > } > > } hello, not sure here - but shouldn't 'strs' be sorted first, to get all the permutations in this case ? ( as in http://www.cplusplus.com/reference/algorithm/next_permutation/ ) -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Gerhard Menzl on 3 Mar 2010 17:23 Daniel T. wrote: > I am pretty dogmatic about the single exit principle. I've had too many > times when I was trying to find some hard to track bug in someone else's > code only to learn that it had to do with some early return or continue > that bypassed a critical piece of code. So you're using C++ without exceptions then? -- Gerhard Menzl Non-spammers may respond to my email address, which is composed of my full name, separated by a dot, followed by at, followed by "fwz", followed by a dot, followed by "aero". [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Daniel T. on 4 Mar 2010 03:09 Karl-Czoh <karczoh(a)googlemail.com> wrote: > > // Answer 2 > > // I told the interviewer that I wasn't sure what he meant by "generate" > > // so I just sent the result to standard output. > > void permutations(const char* in) > > { > > vector<string> strs; > > stringstream ss(in); > > string next; > > while (ss >> next) { > > strs.push_back(next); > > } > > copy(strs.begin(), strs.end(), ostream_iterator<string>(cout, " ")); > > cout << '\n'; > > while (next_permutation(strs.begin(), strs.end())) { > > copy(strs.begin(), strs.end(), > > ostream_iterator<string>(cout, " ")); > > cout << '\n'; > > } > > > > } > > hello, > > not sure here - but shouldn't 'strs' be sorted first, to get all the > permutations in this case ? > ( as in http://www.cplusplus.com/reference/algorithm/next_permutation/ ) Yes you are right, good catch. The examples that came with the problem were all sorted initially, so I took that to be a precondition of the function. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Daniel T. on 4 Mar 2010 03:08 "Balog Pal" <pasa(a)lib.hu> wrote: > "Daniel T." <daniel_t(a)earthlink.net> > > "Balog Pal" <pasa(a)lib.hu> wrote: > > > "Daniel T." <daniel_t(a)earthlink.net> > > > > > > > Balog Pal and I have a different idea of what constitutes DRY in > > > > this case. > > > > > > That should be weird... > > > > It should be weird that programmers disagree about how to write > > code? I think not. > > No, that is pretty common. ;-) (And before we switch context to the > theory field please note my first comment on the first example, that > it is not a big deal -- when the item is such small, there is much > more leeway.) > > But interpreting what "repeat" or "single" means should (IMO) not be > different. I.e if you state, that you like your solution better, and > don't care to apply DRY for this particular case, it is okay for me. > But stating it fits DRY, is not. > > > > > I think that having the extra variables creates two > > > > representations for the piece of knowledge in question > > > > > > There are no extra 'variables' but constants. > > > > > > > (tolower(a) and lo_a both represent the exact same thing.) > > > > > > Huh? One is a transforming function and the other is a *result*. > > > The piece of code is supposed to work on that result. > > > > > > > IMHO, his solution breaks DRY. > > > > > > By *not* repeating the calls that is? Please explain in details > > > how that happens. And how you get to the single point of truth by > > > pasting the algorythm. > > > > I'm not pasting an algorithm, I'm calling a function to obtain a > > property of a type, and calling a function more than once doesn't > > break DRY. > > tolower is IMO not a property and not of a type -- it is a mapping > function. The logic has two places to use th result of that > conversion. Those places are connected to each other, so should use > the same thing instead of a repeated call. > > With so little code the difference may not be evident, but try to > imagine some evolution -- if some aspect changes in coding or > requirements, how many places need a change. > > > The DRY code philosophy is stated as "Every piece of knowledge must > > have a single, unambiguous, authoritative representation within a > > system." > > > > So which is the "single, unambiguous, authoritative representation" > > in this case, tolower(a) or lo_a? Whichever answer you give, you are > > breaking DRY by having the other hanging around. > > I don't get your reasoning. We have the original input character in > 'a' and nowhere else. We then have 'lower cased a' in lo_a. And > nowhere else. (the content of 'a' might be lowercase by some accident > but that does not matter, it is still a different thing.) > > > Storing the result in a variable is a performance optimization, not > > a violation of DRY (IMHO a premature and unnecessary performance > > optimization in this case.) > > tolower() being a pure and constexpr function seem to confusing the > picture. Imagine it is more abstract, and we needed to call some > function in its place, that may do unknown things in the > implementation. > > In that case, I guess, wanting to call it twice would not be so > popular. And most people would prefer my formulation. Since you said you don't get my reasoning, I will try again (hopefully without repeating myself too much :-) In your example, you have two constructs, "tolower(a)" and "lo_a", and the two of them are completely interchangeable. Surely you see that. This interchangability did not come about by accident, they are interchangeable by design, they both represent the same thing. The fact that tolower() is a "pure and constexpr" function isn't confusing the picture, it is a big part of the reason that the two constructs represent the same thing. However you make an important point. If it were the case that "tolower(a)" represented something other than simply "lower case a", if tolower() had side effects for example, then we would be calling the function, in part, to cause those side effects. If that were the case, then tolower(a) and lo_a would *not* represent the same thing and you wouldn't be violating DRY, that isn't the case though. > > > > I am pretty dogmatic about the single exit principle. > > > > > > That would gain you another set of bad marks for in a good C++ > > > shop. SESE works fine for C, not as dogma but as a poor mitigation > > > to lack of RAII. In C++ you shall use RAII fr anything, and SESE > > > is nothing but ilusion anyway as you expect exceptions thrown in > > > the code below the actual code being 'transparent'. > > > > > > And beyond an old dogma grown in a different context, SESE has no > > > benefits only the drawbacks. Read Alexandrescu for more details. > > > > Single exit supports DRY. > > Hm? Those are orthogonal things, better not mixed. > > > With only one return there is only one (authoritative) place that > > the postcondition of the block of code must be checked. > > As you can't check anything in the finction after the return it hardly > has any value -- and if you want to check the postcondition it can be > done at a caller site (as normally done in unit tests) that care > nothing about the number of exits. > > Also, if you want to know the postcondition by just reading the code > (review), it is not at all easier with single exit for too many cases. > Especially if that postcondition have several cases by its nature. > Please take the time to look up AA's examples. > > One classic example is search a thing in a collection. SEME solution > returns right where it is found and have another exit where not found. > The SESE version uses a framework of flags or other obfuscation > measures. It came up in countless many debates and I still waiting tor > a reasonable explanation how it that good. And especially easier to > read or verify. > > > I don't have multiple exits out of the middle of 'if' statements or > > 'while' loops, why should I have them in the middle of functions? > > As I said the normal way is to exit right where you have reached the > postcondition not a mement earlier or later. It may be a single point, > or two or whatever many, and they can happen to be inside an if or > while -- why on earth not? > > And if they are naturally there, making the execution continue, and > smuggle some results to a single point only increase the complexity > and adds opportunity to messing up instead of reducing it. > > > That said, many people have no problem with using break or continue > > to jump out of the middle of a block of code and maybe you are one > > of them. > > Of course. :) And I could add that I'm not afraid of using goto either > if (if!!!) that comes up as the best solution for the case -- just it > is damn rare in C++. > > Believe me, being dogmatic is a bad thing. To go a little back to > originals -- If you were on my interview, a statement like you did, > being dogmatic SESE would map to an automatic "no hire" (after > clarifying you actually mean it). Because I stick to wizard rule #6 -- > my only sovereign is REASON. Dogmaitsm is the very opposite of reason. > I expect programmers to always be aware what they do, and be able to > provide a rationale -- fit for the particular case. Just as guidelines > (any guidelines) are only as good as they are understood -- and > wielder can regress them to roots. (See also "the five whys" from > lean.) > > > I consider it a style issue more than anything else. Some people > > have no problem exiting blocks of code all over the place, I try not > > to. > > You skipped over the my mentioning exceptions. As exceptions can > happen, it is no longer just a style, and if one thinks some layout > *has* the actual SESE property just by restricting to a single return > -- is quite mislead. Thus imposing danger. Here too, I will make another attempt at explaining myself. You have brought up several points and I will attempt to cover them all, but the last item is IMHO the most importnat so I will cover it first. _Exceptions to the Rule_ Of course there are exceptions to every rule (including this one. :-) As programmers we have lots of different design goals and many of them call for mutually opposing solutions, coding is often about trade-offs. It is not wrong of me to avoid multiple exits when there are no important design or performance constraints requiring them. To put it another way, there is no design rule that says "prefer multiple exits from blocks of code," yet that is what you seem to be advocating. (If I am wrong here, then we are probably not that far apart in our positions.) I can see where this issue might have stemmed from the use of the word "dogmatic." I said "pretty dogmatic" and you might have thought that I meant that I insist on single returns no matter what the situation. That is not exactly what I meant, rather I meant that I am more willing to expend the extra effort to find a single return solution than most other programmers. Hopefully the above clears up this issue, but if not then continue reading... _Classic Example, Finding Something in a Collection_ Your "classic example" is IMHO a very good one. When we search for a thing in a collection, should we prefer multiple exits? I think not. This means approaching the problem in a different way, but it doesn't necessarily entail what you called a "framework of flags or other obfuscation measures." If we approach the problem from the assumption that multiple exits are to be preferred, then we might write it like this: template< typename InIt, typename Tp> InIt find(InIt first, InIt last, const Tp val) { for (InIt it = first ; it != last; ++it) if (*it == val) return it; return last; } If we try to avoid multiple returns we might end up with something like this (cribbed from GCC's library): template< typename InIt, typename Tp> InIt find(InIt first, InIt last, const Tp val) { while (first != last && !(*first == val)) ++first; return first; } Now you probably would not write the multiple return example exactly as I did, but however you write it, I don't think that the multiple return example should be preferred as a design rule. I am even willing to add a "result" variable to avoid multiple returns, but I'm not going to throw in a bunch of flags, that would be silly. (As a side note, the GCC library has a separate 'find' for use with random access iterators that does use multiple returns and looks remarkably like a somewhat cleaner version of Duff's Device. This is obviously a performance optimization, which is the one thing that trumps design near the end of a product's construction.) _Postconditions and Where to Check For Them_ Again we come back to not repeating ourselves. If the post condition of a function can be checked by code, then it should be. I think we can agree on that, but where should we do the checking? Your suggestion was that this checking should be done "at a caller site (as normally done in unit tests)." Most functions are complex enough that they require multiple tests, so they are generally called from multiple sites. If we want to put the postcondition checking code in only one place (so we don't repeat ourselves,) then the obvious place to put it is just before the return within the function. If there is more than one return, then we still must repeat ourselves; which is the one thing that we have both agreed we should avoid. Hopefully this post will clear up my thinking about these issues and cover any concerns or confusion you may have. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Daniel T. on 4 Mar 2010 03:09
Gerhard Menzl <clcppm-poster(a)this.is.invalid> wrote: > Daniel T. wrote: > > > I am pretty dogmatic about the single exit principle. I've had too > > many times when I was trying to find some hard to track bug in > > someone else's code only to learn that it had to do with some early > > return or continue that bypassed a critical piece of code. > > So you're using C++ without exceptions then? I don't use exceptions for normal control flow. In my experience (and it might be just in my domain,) if an exception is thrown, then it is because a programmer made an unwarranted assumption while writing some particular piece of code and the code should be fixed so the exception is no longer thrown. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |