2
\$\begingroup\$

I've started learning java. I've written a small program I've done in C before. It's simple input analysis. If there is

  1. number, program checks if number is prime. Then write number: (value of input variable) (prime) for example
  2. date in format DD/MM/YYYY, if does program will print date: DD/MM/YYYY Day (Day represents Monday, ...)
  3. word (plus everything whats not date or number). There program checks, if input isn't palindrome.

So. I would like to ask you, if the code is good or not (and where it isn't). I would like to improve. I'm new in OOP so there may be some mistakes. As I said, I've been working in C before.

package inputtextanalysis;
import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Scanner;
import java.util.Date;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
 *
 * @author Johnczek
 */

public class InputTextAnalysis {

    /**
     * @param args the command line arguments
     */
    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);

        System.out.println("Input Text Analysis. For help type \"-help\", for exiting program type \"exit\"");
        System.out.print("Type text here: ");

        while(true) 
        {
            Detect.detection(sc.next());   
        }
    }

}


class Detect {
    private static int count=0;
    private String s;

    static void detection(String s) 
    {
        if(null != s)
        switch (s) {
            case "exit":
                printExitMessage();
                System.exit(0x0);
            case "-help":
                printHelp();
                break;
            default:
                if(s.matches("^-?\\d+$"))
                {
                    try {
                        if(isPrime(Long.parseLong(s)))
                            printPrimeNumberInfo(s);
                        else
                            printNumberInfo(s);
                    }
                    catch (Exception e) {
                        System.out.println("#ERROR# Long overflow/ underflow");
                        System.out.println("Maximum value: " + Long.MAX_VALUE + " | " + "Minimum value " + Long.MIN_VALUE);
                        System.out.println("Try again please");
                        count--;
                        break;
                    }
                }
                else if(isValidDate(s))
                    printDateInfo(s);
                else
                {
                    if(isPalindrome(s))
                        printPalindromeWordInfo(s);
                    else
                        printWordInfo(s);
                }   count++;
                break;
        }   
    }

    public static void printNumberInfo(String s) 
    {
        System.out.println("number: " + s);
    }
    public static void printPrimeNumberInfo(String s)
    {
        System.out.println("number: " + s + " (prime)");
    }
    public static void printDateInfo(String s)
    {
        try {
            System.out.println("date: " + s + " (" + getDay(s) + ")");
        } catch (ParseException ex) {
            Logger.getLogger(Detect.class.getName()).log(Level.SEVERE, null, ex);
        }
    }
    public static void printWordInfo(String s)
    {
        System.out.println("word: " + s);
    }
    public static void printPalindromeWordInfo(String s)
    {
        System.out.println("word: " + s + " (palindrome)");
    }
    public static void printExitMessage()
    {
        System.out.println("Exiting program. " + count + " objects loaded");
        System.out.println("Thanks for using this program");     
    }
    public static void printHelp()
    {
        System.out.println("============================ Input Text Analysis ============================");
        System.out.println("Program loads data from input. Then prints info about these datas on output");
        System.out.println("In case a) input is number, program checks if the number is prime or not");
        System.out.println("In case b) input is in date type (DD/MM/YYYY) program calculate the day");
        System.out.println("In other case program checks if input is palindrome or not");
        System.out.println("Write \"exit\" for exiting program");
        System.out.println("=============================================================================");
        System.out.println("\n");
        System.out.print("Type text here: ");
    }

    public static boolean isPrime(long number) 
    {
        if (number%2==0 || number=='2' || number=='1')
            return false;
        for(int i=3; i*i<=number;i+=2)
        {
            if(number%i==0)
                return false;
        }
        return true;
    }
    public static boolean isValidDate(String s)
    {
        return s.matches("(^(((0[1-9]|1[0-9]|2[0-8])[\\/](0[1-9]|1[012]))|((29|30|31)[\\/](0[13578]|1[02]))|((29|30)[\\/](0[4,6,9]|11)))[\\/](19|[2-9][0-9])\\d\\d$)|(^29[\\/]02[\\/](19|[2-9][0-9])(00|04|08|12|16|20|24|28|32|36|40|44|48|52|56|60|64|68|72|76|80|84|88|92|96)$)");
    }
    public static boolean isPalindrome(String s)
    {
        for(int i=0, n=s.length()-1; i<n; i++, n--)
        {
            if(s.charAt(i) != s.charAt(n))
                return false;
        }
        return true;
    }

    public static String getDay(String s) throws ParseException
    {
        SimpleDateFormat format1=new SimpleDateFormat("dd/MM/yyyy");
        Date dt1=format1.parse(s);
        DateFormat format2=new SimpleDateFormat("EEEE"); 
        String finalDay=format2.format(dt1);
        return finalDay;
    }
}
\$\endgroup\$
1
\$\begingroup\$

try-with-resources

        Scanner sc = new Scanner(System.in);

        System.out.println("Input Text Analysis. For help type \"-help\", for exiting program type \"exit\"");
        System.out.print("Type text here: ");

        while(true) 
        {
            Detect.detection(sc.next());   
        }

My IDE will complain that you never close the Scanner. If you put the whole thing in a try-with-resources block, it stops complaining.

        try (Scanner sc = new Scanner(System.in)) {
            System.out.println("Input Text Analysis. For help type \"-help\", for exiting program type \"exit\"");
            System.out.print("Type text here: ");

            while (true) {
                Detect.detection(sc.next());
            }
        }

This is a good pattern to learn. It doesn't matter much with a Scanner that you declare in main (whatever my IDE says), but in other circumstances, leaving an unclosed resource can cause memory leaks or other issues.

Note that this is a normal try block otherwise. It could catch an exception or add a finally block. I just didn't need to do so here. The try-with-resources will handle the declared object and close it cleanly if it is open.

In Java, the general standard is to put the { on the same line as its control statement, not alone on its own line. So long as you are consistent in your codebase, it's not a huge deal to do it the other way. But since you aren't consistent here, I'm doing it the "standard" way.

Prefer early exit for exceptional conditions

        if(null != s)

The entire method is inside this if. First, I think that this is exactly the situation where using the single statement form of if is dangerous. It would be easy to forget that everything else is inside the switch statement and add a new statement after the switch.

Second, this is easily avoidable.

        if (null == s) {
            return;
        }

Now it doesn't matter what statements we may add outside the scope of the if. If s is null, we panic immediately. It might be better to throw an exception here, but I'll leave that up to you.

Don't assume

                    catch (Exception e) {
                        System.out.println("#ERROR# Long overflow/ underflow");

You catch a generic exception and then assume that it is a Long overflow or underflow. It would be better to catch a more specific exception (or exceptions) and handle those. Otherwise, you may find that your response makes no sense.

|improve this answer|||||
\$\endgroup\$
0
\$\begingroup\$

In term of OOP I would say that I don't see any OOP principle.

Why not creating a Number class with one static accept(String str):boolean and the printXxxxInfo methods in it. And the same for Date and Sentence. This way, the internals of all classes are grouped in themselves.

default:
  if ( Number.accept(s) ) {
    new Number(s).printInfo(); // If (this.isPrime()) { .. } else { .. }
  } else if ( Date.accept(s) ) {
    new Date(s).printInfo();
  } else {
    new Sentence(s).printInfo();
  }

Much better, you can create an abstract class for your inputs and use it as parent for Number, Date and Sentence and use a factory to create them :

default:
  Input input = Inputs.parse(s); // Factory method for inputs
  input.printInfo();

Also, I prefer to remove the System.out from my classes. This is cleaner and easier to test. So instead of printing the infos to System.out from your inputs, you can ask them the info and print it.

default:
  Input input = Inputs.parse(s);
  System.out.println(input.getInfo());
|improve this answer|||||
\$\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.