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;
 }
}
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Aug 15, 2016 at 18:33
\$\endgroup\$

2 Answers 2

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.

answered Aug 15, 2016 at 19:14
\$\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());
answered Aug 16, 2016 at 6:57
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.