7
\$\begingroup\$

Challenge Story:

"7777...8?!??!", exclaimed Bob, "I missed it again! Argh!" Every time there's an interesting number coming up, he notices and then promptly forgets. Who doesn't like catching those one-off interesting mileage numbers?

Let's make it so Bob never misses another interesting number. We've hacked into his car's computer, and we have a box hooked up that reads mileage numbers. We've got a box glued to his dash that lights up yellow or green depending on whether it receives a 1 or a 2 (respectively).

It's up to you, intrepid warrior, to glue the parts together. Write the function that parses the mileage number input, and returns a 2 if the number is "interesting" (see below), a 1 if an interesting number occurs within the next two miles, or a 0 if the number is not interesting.

Note: In Haskell, we use No, Almost and Yes instead of 0, 1 and 2.

Challenge details:

Interesting numbers are 3-or-more digit numbers that meet one or more of the following criteria:

  • Any digit followed by all zeros: 100, 90000
  • Every digit is the same number: 1111
  • The digits are sequential, incrementing†: 1234
  • The digits are sequential, decrementing‡: 4321
  • The digits are a palindrome: 1221 or 73837
  • The digits match one of the values in the awesomePhrases array

† For incrementing sequences, 0 should come after 9, and not before 1, as in 7890
‡ For decrementing sequences, 0 should come after 1, and not before 9, as in 3210

Error Checking:

  • A number is only interesting if it is greater than 99!
  • Input will always be an integer greater than 0, and less than 1,000,000,000.
  • The awesomePhrases array will always be provided, and will always be an array, but may be empty. (Not everyone thinks numbers spell funny words...)
  • You should only ever output 0, 1, or 2.
const isInteresting = (n, awes) => {
  if (n < 98) return 0;
  if (n < 100) return 1;

  const nums = [n, n + 1, n + 2];

  for (let i = 0; i < nums.length; i += 1) {
    const flipped = [];
    let copy = nums[i];
    let isSame = true;
    let isDecr = true;
    let isIncr = true;

    for (copy; copy > 0; copy /= 10) {
      const curr = copy % 10;

      if (flipped.length) {
        const prev = flipped[flipped.length - 1];

        if (prev !== curr) isSame = false;
        if (prev + 1 !== curr) isDecr = false;
        if (!(prev - 1 === curr || prev + 9 === curr)) isIncr = false;
      }

      flipped.push(curr);

      if (copy - curr === 0) {
        // is awesome could technically be better placed at the beginning
        // of the loop. Looks nicer here though.
        const isAwes = awes.some(phrase => phrase === nums[i]);
        const isSingl = Number(`${flipped.join('')}`) < 10;
        const isPalin = Number(`${flipped.join('')}`) === nums[i];
        const almostOrYes = i > 0 ? 1 : 2;

        if (isSingl || isSame || isDecr || isIncr || isPalin || isAwes) return almostOrYes;
      }

      copy -= curr;
    }
  }
  return 0;
};

About the code: Works and produces desired results

Why I want reviews: Going to begin looking into applying and am brushing up on general problem solving. Want to be as self aware as possible heading into interviews. I also feel fairly inadequate as a programmer so I will attempt to do these often in order to build up momentum.

Background:

I noticed that when I went to check other people's answers to this challenge it was mostly string manipulation and a lot of splitting code into their own functions (needed in some cases, not so much in others). This opinionated line of thinking made me realize I honestly should get feedback on my own code and get a better idea of what someone with more experience would think about both this solution in comparison to the "obvious" solution and as a standalone given the language features used.

What I need:

  1. An honest opinion about the code. Feel free to touch up on anything and everything that you feel is relevant. (You know, the usual review)
  2. Does this code raise any red flags or good points for a potential hire? (Ok if this is not enough to go by)
  3. I am uncertain if you can do this but if possible spoiler tag your code. (Or maybe even link to your code?)
  4. Any refactoring/personal takes on the challenge should be preceded with concepts that I can search for so I can maximize the value and maybe even attempt your solution before I look at your code.
  5. Any article, book, video presentation, podcast, etc. which you think might help me with my coding (if you feel I should know/be aware of something)
  6. How long should this have taken to come up with? (opinion)
  7. Is this code optimal?
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! Fantastic first question, I hope you get some great answers! \$\endgroup\$ – Phrancis May 25 '18 at 18:59
  • \$\begingroup\$ Not worth a full answer, but if (n < 100) return 1; is only possible to execute and be true for n of 99 so just make it an equality check for 99. It's slightly more readable that way. \$\endgroup\$ – Stephan May 25 '18 at 20:13
  • \$\begingroup\$ @Stephan Thanks! It checks for both 98 and 99. If I am correct in my line of thinking, it is probably easier to check for 1 thing (less than 100) than it is to check for both 98 and 99. Cheers! \$\endgroup\$ – user166286 May 26 '18 at 1:38
  • 1
    \$\begingroup\$ No I missed 98 when I was looking at it. What you have is fine, sorry for the confusion! \$\endgroup\$ – Stephan May 26 '18 at 1:39
  • 1
    \$\begingroup\$ As long as it is topical, sure. Just avoid idle talk and you should be fine. \$\endgroup\$ – Phrancis May 26 '18 at 3:34
2
\$\begingroup\$

Any article, book, video presentation, podcast, etc. which you think might help me with my coding (if you feel I should know/be aware of something)


Is this code optimal?

Depends on what optimal means to you. CodeWars culture favors concise, clever usages of the language. Conciseness can clarify code. For example using Array and String iteration & manipulation methods you might avoid the index futzing in for loops.

What makes it sub-optimal is the relative difficulty of reading and understanding the program overall.


Does this code raise any red flags or good points for a potential hire? (Ok if this is not enough to go by)

I see lack of experience. Poor to bad variable names, lack of code structure, magic numbers, non-idiomatic constructs (i += 1 instead of i++).

The code in general is confusing. I really wonder why there is so much digit/number manipulation involving an array. I think it is because the code tries to do everything all at once, but it ends up obscuring everything it does.

I'm not saying the algorithm must be changed. Rather, things discussed below can go a long way toward mitigating difficult code.


I don't expect to see professionally polished code in CodeWars but let's raise that standard:

  • Lazy variable names: isSingl || isDecr || isIncr || isPalin || isAwes. isSingl Really? Ah! Saving valuable time by dropping the "e" in "single"! isPalinis inomprehensible. Are you a Sara Palin fan?
  • Enigmatic varialbe names: num copy
  • The inner for loop needs comments because the what, why of it is not clear. That I have to read all of it to understand any of it is not optimal.
  • Declaring variables inside a loop - hence re-declaring them on every iteration.
  • Given 6 distinct "interesting" number patterns I don't readily identify where these are handled. The default, easiest thing to do would be distinct functions. As it is all intermingled that raises concerns about bug side effects across the pattern code. Even w/out separate functions I should not have to .. what I just said about having to read in detail.
  • What does "return 0, 1, or 2" mean?
  • I don't see checking for range 0 - 1,000,000,000

A number is only interesting if it is greater than 99 ...

But I don't get this sequence. If we're talking about 99 here then say "99".

if (n < 98) return 0;
if (n < 100) return 1;

Unnecessary Obsfucation - flipped array

Checking for incrementing or decrementing sequence should be a dead simple one liner each. Why does it involve an array? In that context what does being flipped mean?


Any refactoring/personal takes on the challenge should be preceded with concepts that I can search for ... How long should this have taken to come up with?

I take CodeWars as an opportunity to research the language. So any given exercise should take as long as it takes to meet your learning goals. DO NOT rush and short-change yourself.

I suggest you incrementally refactor this exercise. It's more about learning to code than learning JavaScript. The point is to see the before and after. You may have several iterations. Do it in stages to see the effects of each thing you do. After all that you may want to rewrite from scratch to see how you've learned to integrate all you've learned. If you think your goal is to make it perfect, you're missing the point.

  • Start reading Code Complete, especially the middle third.
  • Then begin with expressive variable names. Even at the risk of ridiculous verbosity. Hit the other extreme of the spectrum, you got this end nailed. You have to overshoot in order to find the sweetspot.
  • Structure. Always strive for structure. Make separate functions for each number pattern. If anyone tells you "it's not enough code, it's not worth the effort" does not know scheiss from shineola about creating quality maintainable code in the real world.
  • Comments. Tell "why" not "what". Learn to balance comments with expressive, self-documenting code. P.S. submitting code to a potential employer with a comment that repeats the code says "I'm effing incompetent, but at least I'm stupid".
  • After reading that middle third of Code Complete then do a from scratch rewrite.

Edit - replies to comments

I am following the Airbnb style guide and they are pretty explicit on the no ++.

An interesting SO answer on that unary operator

Here is that AirBnB reference. I strongly disagree with the "semicolon auto-insertion" premise. If you are allowing the JS interpreter to auto-insert semicolons you are screwing yourself big time. ALWAYS USE SEMICOLONS TO END YOUR STATEMENTS, period; Don't pretend you can devine what the interpreter will do in every case; and sleep soundly.

As for avoiding the wrong variation, pre-increment when you meant post-increment: Y'all be careful out there, Ya-hear? Seriously, the unary operator is ubiquitous and the gotcha is documented, intended behavior. Try this:

  • read "++", say "add.one."
  • See "++i", say "add.one. (then use) i."
  • See "i++" say "(use) i (then) add.one."
  • You can also say it is consistant with arithmetic operation order.

Also, I updated my question by adding the meaning behind 0,1,2. ... does my addition up top regarding 0,1,2 make things clearer? ... please update answer

Yes it does, but that does not address the problem, which is the code.

let interesting = 2, interestingClose = 1, interestingNot = 0;

Changing the answer would completely destroy the illustration for the future reader. Updating the original question in response to answers is discouraged on StackExchange. Distinct from error corrections, it can cause answers and comments to become wrong, non-sequitur, and confusing overall.

I would say however that additions, distinct from changes, can be OK sometimes; particularly for clarifications requested in comments which are common immediately after postings. But even then, conventional wisdom says to fence an addition with (bolded) "Edit .. endEdit", and a brief "why" is helpful.

Declaring variables inside a loop ... I am under the impression that declaring any higher up than necessary in a scope is discouraged.

That is generally true but inside loops... Well, for compiled languages there is valid reasoning not doing this but a brief browse is telling me it's not an issue in JS. And when creating closures in that loop/block-scope (not applicable here) you need to do this.

end Edit

\$\endgroup\$
  • \$\begingroup\$ Wow! Thank you so much. I wanted to bring a couple of points up to ensure that some of the comments up top still apply. Some of them are minor while others help me out in important ways. In no particular order: About the i++ vs i += 1 . I am following the Airbnb style guide and they are pretty explicit on the no ++. Can you let me know what you think regarding this and maybe update your answer if this sways you at all? github.com/airbnb/… \$\endgroup\$ – user166286 May 26 '18 at 7:22
  • \$\begingroup\$ Thanks for the observation on variable naming: I tend to actually go verbose and in this case I forgot to expand my variable names when I posted here. You are completely right though. Any code others will read should be clearer through variable names as well. Also, I updated my question by adding the meaning behind 0,1,2. Would it be possible to update your answer to reflect this? \$\endgroup\$ – user166286 May 26 '18 at 7:57
  • \$\begingroup\$ I noticed you mention that you did not understand the sequence i used, given that "...a number is only interesting if it is larger than 99" does my addition up top regarding 0,1,2 make things clearer? If so, could you update your answer to reflect this? Thanks! \$\endgroup\$ – user166286 May 26 '18 at 8:05
  • \$\begingroup\$ "Declaring variables inside a loop - hence re-declaring them on every iteration." - Given that I would have to re-initialize them every time the loop iterates anyway, would it not make more sense to have the loop just do it for me instead of me having to explicitly do it at the start of the loop after a declaration anyway? I am under the impression that declaring any higher up than necessary in a scope is discouraged. In this case, the variables are needed in the inner loop and are declared in the outer loop because they must be initialized again for every run of the outer loop. \$\endgroup\$ – user166286 May 26 '18 at 8:34
  • \$\begingroup\$ Fantastic! So I actually agree with you as well on the unary operator. It is ubiquitous and people should understand instead of avoiding it. I do however, disagree with the automatic semicolon insertion point. There are maybe three circumstances in which you might see unexpected behavior and they can all be avoided by following some simple rules which can be set up on a linter. But of course, that is a conversation for another time! \$\endgroup\$ – user166286 May 27 '18 at 12:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.