Matching program in Java

Question

I am new to Java and wrote one small program. It works fine, but it looks like it doesn't meet OOP concepts. Could someone look and give me advice so that I can fine tune?

public class App {
 public static void main(String[] arg) {
 String str = new String(
 "]d010036195815011121123456789]d17YYMMDD1012345678901");
 String matchItemAI = new String("01"); //GTIN-14
 String matchSNoAI = new String("21"); //Serial Number
 String matchExpDtAI = new String("]d17");// Expiry Date
 String matchLotNoAI = new String("10"); //Lot Number
 //Company Prefix
 String matchCompPrefixUS = new String("03"); //US Company Prefix
 String matchCompPrefixCork = new String("03"); //US Company Cork
 String matchCompPrefixSKorea = new String("03"); //US Company South Korea 
 String value = str;
 String value1 = new String();
 String value2 = new String();
 String value3 = new String();
 char ch;
 int pos;
 // 1. Need to print ]d0100361958150111 like that 61958-1501-1
 // 2. Need to print 21123456789 like that 123456789
 // 3. Need to print ]d17YYMMDD like that YYMMDD
 // 4. Need to print 1012345678901 like that 12345678901
 // GS1 Start with this String....It's a GS1 2d Bar Code, Confirmed Then
 // Process the record
 if (str.startsWith("]d")) {
 System.out.println("GS1 2D Input String :" + str);
 ch = str.charAt(2);
 switch (ch) {
 case '0':
 System.out
 .println("Calculating Unit of Sale (0) Packaging Indicator digits for GTIN 14's : "
 + str.charAt(2)); 
 pos = str.indexOf(matchItemAI);
 System.out.println("GS1 pos:" + value.substring(pos)+" POS Value " +pos);
 value = value.substring(pos + 5, value.length());
 value1 = value.substring(0, 5);
 value2 = value.substring(value1.length(), value1.length() + 4);
 value3 = value.substring(value1.length() + value2.length(),
 value1.length() + value2.length() + 1);
 value3 = value1 + "-" + value2 + "-" + value3;
 value = value.trim();
 System.out.println("Found string Item : " + value3);
 // /GET Serial Number
 pos = str.indexOf(matchSNoAI); // AI 21
 // System.out.println("Found string SNo : " + pos);
 value = str.substring(pos + 2, str.lastIndexOf(matchExpDtAI)); //pos + 2 + 9);
 value = value.trim();
 System.out.println("Found string SNo : " + value);
 pos = str.lastIndexOf(matchExpDtAI);
 // System.out.println("Found string Expiry Date " + pos);
 value = str.substring(pos + 4, pos + 4 + 6);
 value = value.trim();
 System.out.println("Found string Expiry Date : " + value);
 pos = str.lastIndexOf(matchLotNoAI);
 // System.out.println("Found string Lot Number " + pos);
 value = str.substring(pos + 2, pos + 2 + 11);
 value = value.trim();
 System.out.println("Found string Lot Number : " + value);
 break;
 case '3':
 System.out
 .println("Calculating Bundle/Multipack 3 Packaging Indicator digits for GTIN 14's : "
 + str.charAt(2));
 break;
 case '5':
 System.out
 .println("Calculating Shipper 5 Packaging Indicator digits for GTIN 14's : "
 + str.charAt(2));
 break;
 default:
 System.out
 .println("Error - invalid selection entered! for Multipacking ");
 break;
 }
 }
 }
}

---

Here I made change as per your suggestion, can you please review and let me know any other suggestion ?

public class App {
public static void main(String[] arg) {
 String str = new String(
 "]d010036195815011121123456789]d17YYMMDD1012345678901");
 String matchItemAI = "01"; //GTIN-14
 String matchSNoAI = "21"; //Serial Number
 String matchExpDtAI = "]d17";// ExpiryDate
 String matchLotNoAI = "10"; //Lot Number
 String WitoutFNC1 = str;
 String NDCCode = "";
 String itemRef = "";
 String itemNo = "";
 int pos;
 // GS1 Start with this String....It's a GS1 2d Bar Code, Confirmed 
 if (str.startsWith("]d")) {
 System.out.println("GS1 2D Input String :" + str);
 switch (str.charAt(2)) {
 case '0':
 pos = str.indexOf(matchItemAI);
 System.out.println("GS1 pos:" + WitoutFNC1.substring(pos)+" POS Value " +pos);
 WitoutFNC1 = WitoutFNC1.substring(pos+5, WitoutFNC1.length());
 NDCCode = WitoutFNC1.substring(0, 5);
 itemRef = WitoutFNC1.substring(NDCCode.length(), NDCCode.length() + 4);
 itemNo = WitoutFNC1.substring(NDCCode.length() + itemRef.length(),NDCCode.length() + itemRef.length() + 1);
 itemNo = String.format("%s-%s-%s",NDCCode, itemRef ,itemNo);
 WitoutFNC1 = WitoutFNC1.trim();
 System.out.println("Found string Item : " + itemNo);
 // /GET Serial Number
 pos = str.indexOf(matchSNoAI); // AI 21
 // System.out.println("Found string SNo : " + pos);
 WitoutFNC1 = str.substring(pos + 2, str.lastIndexOf(matchExpDtAI)); //pos + 2 + 9);
 WitoutFNC1 = WitoutFNC1.trim();
 System.out.println("Found string SNo : " + WitoutFNC1);
 pos = str.lastIndexOf(matchExpDtAI);
 // System.out.println("Found string Expiry Date " + pos);
 WitoutFNC1 = str.substring(pos + 4, pos + 4 + 6);
 WitoutFNC1 = WitoutFNC1.trim();
 System.out.println("Found string Expiry Date : " + WitoutFNC1);
 pos = str.lastIndexOf(matchLotNoAI);
 // System.out.println("Found string Lot Number " + pos);
 WitoutFNC1 = str.substring(pos + 2, pos + 2 + 11);
 WitoutFNC1 = WitoutFNC1.trim();
 System.out.println("Found string Lot Number : " + WitoutFNC1);
 break;
 case '3':
 System.out
 .println("Calculating Bundle/Multipack 3 Packaging Indicator digits for GTIN 14's : "
 + str.charAt(2));
 break;
 case '5':
 System.out
 .println("Calculating Shipper 5 Packaging Indicator digits for GTIN 14's : "
 + str.charAt(2));
 break;
 default:
 System.out
 .println("Error - invalid selection entered! for Multipacking ");
 break;
 }
 }
 }
}

• syb0rg: "but it's not like it should be" - does this mean that it does not work as intended?
• user3246453: it' working as it is but looks like it's not meeting OOP concept
• syb0rg: I see. Would you mind re-wording that phrase in your question? It could confuse some people to believe that your code does not work as intended, and therefore is off-topic.
• ChrisW: Please don't edit the original code, because that would invalidate the existing answer.
• ChrisW: @user3246453 I edited your question to add your new code after (not instead of) the original code. Or, instead of adding to an existing question, it's also possible to ask a new question: see How to deal with follow-up questions?

Answer 1

1. When you look at the color highlighting of your code, you can see that WithoutFNC1 is colored blue like other Java types. This also suggests that in the Java world, it is not recommended for non-static variable names to start with an upper case.

2. These two blocks are very similar and you can already create a static method for it, for starters:

    pos = str.lastIndexOf(matchExpDtAI);
    // System.out.println("Found string Expiry Date " + pos);
    WitoutFNC1 = str.substring(pos + 4, pos + 4 + 6);
    WitoutFNC1 = WitoutFNC1.trim();
    System.out.println("Found string Expiry Date : " + WitoutFNC1);
    pos = str.lastIndexOf(matchLotNoAI);
    // System.out.println("Found string Lot Number " + pos);
    WitoutFNC1 = str.substring(pos + 2, pos + 2 + 11);
    WitoutFNC1 = WitoutFNC1.trim();
    System.out.println("Found string Lot Number : " + WitoutFNC1);
   

3. Also, when you say a substring is "found", do you really mean that the string is not empty? Or perhaps you can apply other validation to ensure that the desired substring is really found? You can create a static method to test your "found" substrings. Otherwise, I suppose deriving an empty substring and printing Found string Item : is not going to be helpful...
4. Just a simple note on most methods for String: If you find yourself repeating variable assignment, you can string one method after another, e.g. " this is some text ".substring(5).trim(), because they simply return copies of the String.

Answer 2

• You consistently do String foo = new String("some string"). This is useless, as "some string" already is a string. Simplify your code to String foo = "...".

• You declare some variables far before you use them. Try to declare your variables as close to their point of use as possible, e.g. instead of

  char ch;
  ...
  ch = str.charAt(2);
  

  you could just do char ch = str.charAt(2). The same holds true for pos, value, value1, value2, value3.

• Actually, you don't even need the ch variable as you could do switch (str.charAt(2)) { ... } directly.

• Some of your variables have very bad named: value1 does not communicate any intent or meaning. Other variables use unecessary abbreviations.

• You have certain magic numbers that could be replaced. E.g. in str.substring(pos + 4, pos + 4 + 6), the 4 is actually matchExpDtAI.length(). I have no idea where the 6 comes from.

• This code is pure obfuscation:

  value = value.substring(pos + 5, value.length());
  value1 = value.substring(0, 5);
  value2 = value.substring(value1.length(), value1.length() + 4);
  value3 = value.substring(value1.length() + value2.length(),
   value1.length() + value2.length() + 1);
  value3 = value1 + "-" + value2 + "-" + value3;
  value = value.trim();
  System.out.println("Found string Item : " + value3);
  

  Note that you don't use the value1, value2, and value3 variables outside of this snippet, so they are unecessary. If we count the lengths of those substrings, we can see that this code should be equivalent:

  value = value.substring(pos + 5, value.length());
  System.out.println("Found string Item : " +
   value.substring(0, 5 ) + "-" +
   value.substring(5, 5 + 4 ) + "-" +
   value.substring(5 + 4, 5 + 4 + 1));
  value = value.trim();
  

  Those constant offsets can be folded, which makes it easier to see that these substrings are actually consecutive:

  value = value.substring(pos + 5), value.length());
  System.out.println("Found string Item : " +
   value.substring(0, 5) + "-" +
   value.substring(5, 9) + "-" +
   value.substring(9, 10));
  value = value.trim();
  

• Actually, let's use String.format for that:

  value = value.substring(pos + 5, value.length());
  System.out.println(String.format("Found string Item: %s-%s-%s",
   value.substring(0, 5),
   value.substring(5, 9),
   value.substring(9, 10)
  ));
  value = value.trim();
  

Before thinking about whether an object-oriented approach would be sensible (it isn't), you have other parts of your code to clean up first.

• user3246453: Amon, Thanks for reviwing the code and your valuable suggestion and feedback, I will make change as you suggested, it's really helpful for me, please keep doing good work