SPOJ Adding reversed number in Java

Question

I just started to practice for competitive programming and just started to solve the most solved problems on SPOJ.

This is what I made for ADDREV: Adding reversed numbers in Java (https://www.spoj.com/problems/ADDREV/)

The Antique Comedians of Malidinesia prefer comedies to tragedies. Unfortunately, most of the ancient plays are tragedies. Therefore the dramatic advisor of ACM has decided to transfigure some tragedies into comedies. Obviously, this work is very hard because the basic sense of the play must be kept intact, although all the things change to their opposites. For example the numbers: if any number appears in the tragedy, it must be converted to its reversed form before being accepted into the comedy play.

Reversed number is a number written in arabic numerals but the order of digits is reversed. The first digit becomes last and vice versa. For example, if the main hero had 1245 strawberries in the tragedy, he has 5421 of them now. Note that all the leading zeros are omitted. That means if the number ends with a zero, the zero is lost by reversing (e.g. 1200 gives 21). Also note that the reversed number never has any trailing zeros.

ACM needs to calculate with reversed numbers. Your task is to add two reversed numbers and output their reversed sum. Of course, the result is not unique because any particular number is a reversed form of several numbers (e.g. 21 could be 12, 120 or 1200 before reversing). Thus we must assume that no zeros were lost by reversing (e.g. assume that the original number was 12).

Input

The input consists of N cases (equal to about 10000). The first line of the input contains only positive integer N. Then follow the cases. Each case consists of exactly one line with two positive integers separated by space. These are the reversed numbers you are to add.

Output

For each case, print exactly one line containing only one integer - the reversed sum of two reversed numbers. Omit any leading zeros in the output.

import java.util.*;
import java.lang.*;

class Main
{

public static void main (String[] args) throws java.lang.Exception
{
    Scanner sc = new Scanner(System.in);
    sc.nextLine();
    while(sc.hasNextInt())
    {
        printNextNumber(sc);
    }
    
}

public static void printNextNumber(Scanner sc)
{
    int a = sc.nextInt();
    int b = sc.nextInt();
    int reversedA = reverseNumber(a);
    int reversedB = reverseNumber(b);
    int reversedSum = reverseNumber(reversedA+reversedB);
    System.out.println(reversedSum);
}

public static int reverseNumber(int number)
{
    return listToNumberMadeOfDigits(numberToListOfDigits(number));
    
}

public static List<Integer> numberToListOfDigits(int number)
{
    List<Integer> list = new ArrayList<>();
    while(number>0){
        list.add(number%10);
        number /= 10;
    }
    
    Collections.reverse(list);
    return list;
}

public static int listToNumberMadeOfDigits(List<Integer> digits)
{
    int number = 0;
    for(int i = 0; i < digits.size();i++){
        number += Math.pow(10,i)*digits.get(i); 
    }
    return number;
}

 }

How is this code? What should I avoid in the future?

Answer 1

First, note that this review is of general good coding practices for production code, and not all suggestions will be appropriate for the sphere of "competitive programming", in which I have no significant experience.

Some thoughts:

For readability, use whitespace consistently and more frequently. There should be whitespace (0) after control statements (if, while, for), around operators (+, %, ..), and before curly braces ({).

In Java, the convention is that open curly braces go on the same line, not on a newline.

Use final aggressively when variable declarations will not change to reduce cognitive load on readers.

AutoCloseable resources such as Scanner should be used in a try-with-resources block to ensure they get closed.

a and b are poor choices for variable names because they're meaningless. Variable names should describe the value they hold. You probably don't even need the intermediate variables.

Methods not intended for use outside their class should be private. Always prefer the most restrictive access you can reasonably apply.

In a real application, it would be highly preferable to having the reversed sum pushed back to the main() method to handle printing. This gives you flexibility, rather than pushing your output choice through your code - if you want to change later, it changes in fewer, higher-level places.

It might be nice to use the number of cases they provide you with.

Don't throw Exception ever - throw the most specific checked exception possible. Don't declare thrown checked exceptions if you don't actually throw them.

Your algorithm is much more complex than it needs to be. Lists are extraneous. The problem can be solved with math.

If you were to apply all the changes I suggested, your code might look more like:

import java.util.Scanner;

class Main {

    public static void main(final String[] args) {
        try (final Scanner sc = new Scanner(System.in)) {
            final int numberOfCases = sc.nextInt();
            for (int i = 0; i < numberOfCases; i++) {
                final int firstNumber = sc.nextInt();
                final int secondNumber = sc.nextInt();
                System.out.println(addReversedNumbers(firstNumber, secondNumber));
            }
        }
    }

    private static int addReversedNumbers(final int firstNumber, final int secondNumber) {
        return reverseNumber(reverseNumber(firstNumber) + reverseNumber(secondNumber));
    }

    private static int reverseNumber(final int number) {
        int numberToReverse = number;
        int reversedNumber = 0;

        while (numberToReverse > 0) {
            final int digit = numberToReverse % 10;
            numberToReverse /= 10;
            reversedNumber = 10 * reversedNumber + digit;
        }
        return reversedNumber;

    }

}

Answer 2

Separate responsibilities cleanly

Which function's responsibility is it to parse the input? In the posted code this is shared by main and printNextNumber. main parses the number of test cases and loops over them, and lets printNextNumber parse the individual test cases.

It's confusing when a clear responsibility is split among multiple functions, each doing only part of it. Why? These functions share objects and state (scanner), therefore while working on any of these functions, you have to keep in mind what impact the changes may have on the other. This is really too much of a mental burden, it would be a lot easier if you could focus your attention on function, without worrying about another.

The parsing of the input would have been better all in main. Especially when I look at a function named printNextNumber, I don't expect any parsing to happen there, only printing.

Adding reversed numbers

Why are the numbers reversed? What's the point? Is the goal really to reverse numbers so you could add them and then reverse the result? Maybe it is, but I doubt it.

Imagine if you had to add arbitrary large integers, so large that they don't fit in int, neither a long, for example 12345678901234567890 and 99999999999999999999999999. How would you go about adding them? How would you do it without a computer, only pen and paper? You would start from the end, and work backwards.

In a program, if the numbers are given reversed as string, and you have to return them in reverse and as string, then it's actually quite convenient for performing the addition digit by digit, appending the added digits to a StringBuilder. A solution is possible without reversing digits, and supporting arbitrarily large numbers. I suggest to try that way, as an exercise. That is, implement this function:

private static String addReversedNumbers(String firstNumber, String secondNumber) {
    // TODO
}

Which can be used by main as such:

public static void main(String[] args) {
    try (final Scanner sc = new Scanner(System.in)) {
        final int numberOfCases = sc.nextInt();
        for (int i = 0; i < numberOfCases; i++) {
            final String firstNumber = sc.next();
            final String secondNumber = sc.next();
            System.out.println(addReversedNumbers(firstNumber, secondNumber));
        }
    }
}

Answer 3

You can avoid the call to Collections.reverse by using a structure that allows for efficient additions to the front:

public static List<Integer> numberToListOfDigits2(int number)
{
    // LLs allow for fast head additions
    LinkedList<Integer> list = new LinkedList<>(); 

    while(number > 0) {
        list.addFirst(number % 10); // Note the change here
        number /= 10;
    }

    return list;
}

This prevents numberToListOfDigit from needing to reiterate the whole list at the end to reverse it. For small lists, the overhead will be minimal, but it's worth thinking about.

I also spaced out your > and % calls for readability. You have everything (inconsistently) compacted in your other function as well. I'd change listToNumberMadeOfDigits to something closer to:

public static int listToNumberMadeOfDigits(List<Integer> digits)
{
    int number = 0;

    for(int i = 0; i < digits.size(); i++) {
        number += Math.pow(10, i) * digits.get(i);
    }

    return number;
}

I like space. I find it helps readability in nearly every situation.