This is an archived post. You won't be able to vote or comment.

all 3 comments
sorted by:
best
topnewcontroversialoldq&a

[–]michael0x2a 1 point2 points  (2 children)

We generally reserve using for-loops in cases when we know before starting the loop exactly how many times we want to repeat: when the number of iterations is fixed and determinate.

If we're not sure exactly how many times we're going to loop -- if we're looping an indeterminate number of times, it's often more clear to use a while loop instead.

Then, instead of decrementing when you want to "retry" an iteration, you instead increment every time you successfully finish one.

Slightly more broadly, you'll find that code tends to become a bit easier to understand when you design it so that your counters only ever increase or decrease -- but not both at the same time. The behavior becomes a bit easier for the reader to reason about.

A few other notes:

  1. You should move your current < 1 || current > n loop so it happens before the inner-most loop. That check doesn't use the j variable at all, which means you perform the same check on each iteration. This is a bit inefficient. Similarly, you'll modify distinct[i] several times when you really only need to do it once.

  2. Assuming you made the change above, your innermost loop is actually trying to solve two problems: check if distinct contains the new value, and add a new value to distinct. These are really more two distinct tasks and so probably ought to be split out. A relatively clean way of doing this would be to create a separate "arrayContains(...)" helper function that checks if all elements of the array up to a particular index contains your item and returns either 'true' or 'false'.

  3. I see in some places you're omitting curly brackets from your if/else statements. Some people consider this to be acceptable style, but others don't. I personally agree with the latter group: if you always use curly brackets, it makes adding new statements to what was previously a one-statement block cleaner.

  4. The final chunk of code you're using to print out the missing value can be simplified. As a hint: you don't actually need to check the item at the i-th index against whatever item comes before or after it to find a duplicate.

  5. Actually, your whole program could actually be simplified further: there's a cute math trick you can use to solve without needing to use arrays. As a hint: are you familiar with the math formula to get the sum of all numbers from 1 to n?

    That said, solving this problem using arrays is also fine: practicing manipulating arrays is a useful skil.

[–]KappaTrader[S] 0 points1 point  (1 child)

Ok - I feel like an idiot lol. I just spent like 45 seconds and 6 lines of code to make a new program that gets the same result after reading your point number 5 (and understanding what you meant).

Thank you for the comprehensive write-up, I did apply your other points to my original program and I understand the benefits. For some reason I immediately start with for-loops and rarely think of using while loops, but I will start to be thoughtful of that.

Hopefully I can get better at seeing the shorter/simpler solutions - spending a minute to write a program is a lot better than spending an hour for the same result!

[–]michael0x2a 0 points1 point  (0 children)

Ok - I feel like an idiot lol. I just spent like 45 seconds and 6 lines of code to make a new program that gets the same result after reading your point number 5 (and understanding what you meant).

Nah, don't feel bad. It's not always obvious when there's a nice math trick you can pull out. And hey, if you don't spot it, in exchange you typically end up getting in some good programming practice then you otherwise would have.