1
\$\begingroup\$

The following code would strip 0 (all numbers ending with 0, it can contain 0, eg: 105 can be is valid but 150 should be eliminated) and return a number in decimal series if all numbers ending with 0 were stripped.
Example : 0 -> 1, 10 -> 11, 19 -> 22.
I would just appreciate any code reviews.

  public static int getStrippedNumber(int num) {
    int setId = (num / 10) + 1;
    int newNum = num + setId;                             
    int newNumSetId = (newNum / 10) + 1;                   
    int numberToReturn = num + newNumSetId;
    return numberToReturn % 10 ==0 ? numberToReturn + 1 : numberToReturn;
    }
\$\endgroup\$
  • 6
    \$\begingroup\$ Your question is unclear. Why 19 -> 21? What is the expected output for 100 and 998 and why? \$\endgroup\$ – Anirban Nag 'tintinmj' Aug 24 '13 at 20:28
  • \$\begingroup\$ Running this code in my head gives 0 -> 2, 10 -> 14, and 19 -> 23. You should post here once you have working code so we can help you improve it. Non-working code is for Stack Overflow. \$\endgroup\$ – David Harkness Aug 24 '13 at 20:32
  • 2
    \$\begingroup\$ There are no 0 in 19 then why it increases to 22? \$\endgroup\$ – Anirban Nag 'tintinmj' Aug 24 '13 at 20:40
  • 4
    \$\begingroup\$ This ideone result may help future reviewer. \$\endgroup\$ – Anirban Nag 'tintinmj' Aug 24 '13 at 21:09
  • 1
    \$\begingroup\$ From the ideone from @tintinmj I could finally guess the intent of the code. But from the same result I can only conclude it has a bug. Note that the output for 198 is 221, and for 199 it is also 221. \$\endgroup\$ – bowmore Aug 25 '13 at 7:20
7
\$\begingroup\$

I'll start by stating what I think your intent is, as it's not 100% clear from your question.

Imagine the series that consists of all positive natural numbers without multiples of 10 : ℕ \10ℕ, in order. Write a function that, given a 0-based index, returns the number at that index in the series

This is what seems to correspond most, with what you have described and what your code does. However there is a discrepancy between what you say your code does, and what it actually does. According to the semantics 10 should map to 12, and this is what your code does, yet as an example you say 10 should map to 11.

The code you have does not seem to fulfill its contract (i.e. it has a semantical bug) it starts misbehaving for iputs higher than 198. 198 gives 221 and 199 also gives 221.

Or, of course, I am way off track...

Code that does fulfill the contract I describe above is actually fairly simple.

public static int getStrippedNumber(int num) {
    return num + num/9 + 1;
}

Borrowing from @tintinmj I have also submitted a ideone to demonstrate

Of course naming this function more appropriately and having a clearer contract explanation are major points of attention. Imagine a developer trying to maintain the code, and all he has is your code and what you have documented.

So improved this would be :

/**
 * Determines the number in the series ℕ\10ℕ (the natural numbers without multiples of 10) at the given index.
 * @param index zero-based index
 * @return the number in the series of ℕ\10ℕ at the given index.
 */
public static int getNwithout10NAtIndex(int index) {
    if (index < 0) {
        throw new IllegalArgumentException();
    }
    return index + index/9 + 1;
}
\$\endgroup\$
7
\$\begingroup\$

Well... Sorry to say this, but it's crappy!

  • The function name doesn't explain what the function does (and using your explanation above for Javadoc would only make things worse, judging from the confusion in the comments)
  • Even knowing the implementation doesn't help, it's hard to tell if the steps performed inside the function correspond to a real-world concept or represent some algorithm
  • it's not clear if this function makes sense for negative values, I'm pretty sure it would return unexpected values (i.e. output for x and -x would be drastically different). If the function is not defined for negative values, throw an IllegalArgumentException when num < 0
  • You use confusing names for variables; I think even a, b, c etc. would be better than num, newNum, numberToReturn (nb. this last one is additionally misleading as you still process it before returning)
  • The function seems to solve some general problem but it uses arbitrary depth: repeats the same operation twice (in lines 1&2 and 3&4) instead of using a loop or recursion (which would also communicate your intentions better)

I know this is the part where you give your recommendations but I believe this function is broken beyond repair until you make it clear what its purpose and contract are. Remember that most real-world mathematical problems are already solved and have well-described algorithms, using well-tested solutions is always a better idea than trying to reinvent the wheel.

\$\endgroup\$
  • \$\begingroup\$ function is broken beyond repair ? \$\endgroup\$ – JavaDeveloper Aug 25 '13 at 2:13
3
\$\begingroup\$

Your question and output differs. Your logic is not well defined but I can't downvote your question cause it is a working code... sigh!

You said:

The following code would strip 0 (all numbers ending with 0, it can contain 0, eg: 105 is valid but 150 should be eliminated)

But by running your code 105 goes to 117 WHY?

However from the question title (code to strip 0 from decimal series) and from the problem description(misleading) I think

 public static int getStrippedNumber(int num) {
     return (num + 1) % 10 == 0 ? (num + 2) : (num + 1);
 }

is ENOUGH!!!

Since you didn't say anything about negative numbers, I'm assuming you didn't think about it. Go with @kryger's answer throw an IllegalArgumentException for negative numbers.

\$\endgroup\$

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.