-2
\$\begingroup\$

This is my code written to check whether a given number is armstrong or not . but the logic differs from the conventional logic of using integer and separating its digits and calculating , instead i optimize the logic of separating the digits using Sting and char array . Please review my code ?

import java.util.Scanner;
public class ArmstrongNumberChecker {
    public static void main(String[] args) {
        int number;
        int sum = 0;
        Scanner scanner = new Scanner(System.in);
        try {
            System.out.println("\nEnter the number to check if it is armstrong number\n");
            number = scanner.nextInt();
            String s = Integer.toString(number);
            char[] c = s.toCharArray();
            for (int i = 0; i < s.length(); i++) {
                sum = ( int ) (sum + Math.pow((c[i] - 48), c.length));
            }
            if (sum == number) {
                System.out.println("Number " + number + " is Armstrong");
            } else {
                System.out.println("Number " + number + " is not Armstrong");
            }
        } catch (Exception e) {
            System.out.println("Invalid data");
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to CodeReview! Can you provide a link or a definition of what Armstrong means? \$\endgroup\$ – Austin Hastings May 3 at 1:42
  • \$\begingroup\$ Ya sure Austin, en.m.wikipedia.org/wiki/Narcissistic_number \$\endgroup\$ – Akshay soni May 3 at 2:34
  • 1
    \$\begingroup\$ Welcome to Code Review! I rolled back your last edit. After getting an answer you are not allowed to change your code anymore. This is to ensure that answers do not get invalidated and have to hit a moving target. If you have changed your code you can either post it as an answer (if it would constitute a code review) or ask a new question with your changed code (linking back to this one as reference). Refer to this post for more information \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ May 3 at 6:03
  • \$\begingroup\$ Now what can I do, I am new to code review. \$\endgroup\$ – Akshay soni May 3 at 6:09
  • \$\begingroup\$ Thanks to edit my question to previous, @Sᴀᴍ Onᴇᴌᴀ . \$\endgroup\$ – Akshay soni May 3 at 6:25
3
\$\begingroup\$

It's one big static main method that works only on data from System.in. Start by refactoring the algorithm into a reusable utility method that works with integers. You're working with numbers so requiring the input to be a string is not an improvement (it seems like a cop-out to make the coding easier for you).

An armstrong number can not be larger than 4 * 9^3 so you should add range checks to avoid useless checking.

You're not prepared for negative values.

You catch generic Exception, ignore it and report a pretty useless error message. You know what the possible exceptions are (IOException and NumberFormatException), so catch them and tell the user exactly what went wrong.

\$\endgroup\$
  • 2
    \$\begingroup\$ If you didn't want feedback, why on earth did you post to code review? You should have said that up front and saved everyones time. \$\endgroup\$ – TorbenPutkonen May 2 at 9:50
0
\$\begingroup\$

Thanks for your reviews everyone. Using the ideas in answer I rewrote my code.

Here I changed my void main () and use it to only call my methods to run the program.

public static void main(String[] args) {
        new ArmstrongNumberChecker().mainMethod();
    }

I also separated the tasks by creating methods in my class and handled exceptions in the method .

private void mainMethod() {
        try {
            System.out.println("\nEnter the number to check if it is armstrong number\n");
            number = scanner.nextInt();
            armstrongChecker();
        } catch (InputMismatchException e) {
            System.out.println(e + "\t : Only integers allowed");
        }
    }

    private void armstrongChecker() {
        String s = Integer.toString(number);
        char[] c = s.toCharArray();
        for (int i = 0; i < s.length(); i++) {
            sum = ( int ) (sum + Math.pow((c[i] - 48), c.length));
        }
        if (sum == number) {
            System.out.println("Number " + number + " is Armstrong");
        } else {
            System.out.println("Number " + number + " is not Armstrong");
        }
    }

Putting it all together my code looks like this.

import java.util.InputMismatchException;
import java.util.Scanner;

public class ArmstrongNumberChecker {
    private int number;
    private int sum = 0;
    private Scanner scanner = new Scanner(System.in);

    public static void main(String[] args) {
        new ArmstrongNumberChecker().mainMethod();
    }

    private void mainMethod() {
        try {
            System.out.println("\nEnter the number to check if it is armstrong number\n");
            number = scanner.nextInt();
            armstrongChecker();
        } catch (InputMismatchException e) {
            System.out.println(e + "\t : Only integers allowed");
        }
    }

    private void armstrongChecker() {
        String s = Integer.toString(number);
        char[] c = s.toCharArray();
        for (int i = 0; i < s.length(); i++) {
            sum = ( int ) (sum + Math.pow((c[i] - 48), c.length));
        }
        if (sum == number) {
            System.out.println("Number " + number + " is Armstrong");
        } else {
            System.out.println("Number " + number + " is not Armstrong");
        }
    }
}
\$\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.