Can this be made more efficient?

Can this be made more efficient?

• Bob Hicks

  Thank you Peter.

  If I understand you correctly, you're saying that the original VB 6.0 code should be retained

  Then there is no way to shorten this code

  
  
  

• vitagoni

  When you compare two things, they have to be the same type. The ultimate type of the expression must be bool.

  Therefore, comparisons are written if ( a == b || a == c ) …

  If the IDE editor doesn’t do what you want (it is usually powerful enough for most anything), use Word. E.g., I would change the VB text to this:

  if (strDataField == "" )
  || (strDataField == " " )
  || (strDataField == "0 " )
  || (strDataField == "00 " )
  || (strDataField == "000 " )
  || (strDataField == "0000 " )
  || (strDataField == "00000 " )
  || (strDataField == "000000 " )
  || (strDataField == "0000000 " )
  || (strDataField == "00000000 " )
  || (strDataField == "000000000" )
  || (strDataField == " 00000000" )
  || (strDataField == " 0000000" )
  || (strDataField == " 000000" )
  || (strDataField == " 00000" )
  || (strDataField == " 0000" )
  || (strDataField == " 000" )
  || (strDataField == " 00" )
  || (strDataField == " 0" )
  || (strDataField == "0 0 0 0 0" )
  || (strDataField == " 0 0 0 0 " ) {
  strDataField == " ";

  }
  else {
  strDataField == ValidatePostalCode(strDataField);

  }

  
  
  

• ckramer

  The old VB code does work in the C# app. I've got hundreds of disks (CD and 3.5) I can use for testing. We hold them for 5 years before destroying them.
  
  

• Marcello Savorani

  According to the documentation in the VB code, any zip code that contained a space in any part of it was to be considered as invalid. The data this comes from is usually entered manually at some point and the users wanted to make sure to get every possible invalid entry. We also have to account for Canadian postal codes and European codes.

  I read the comments as :

  "If the ZIP code contains spaces in ANY part and the first five digits (ne: characters) contain all zeroes then the ZIP is considered to be invalid. So, if we read a ZIP code that looks like this:

  "10000" and followed by four spaces, then it is a valid ZIP; however these would be considered as invalid:

  "000000000" <== entire zip is zeroes
  "00000" <== first five characters are zeroes
  " 0000" <== first character is a space
  "0000 " <== last character is a space
  "12 34" <== a space is embedded in the middle
  
  and so on.

  BTW, if you hold the Shift key and press enter, the cursor returns one line instead of two, found that one by accident.

  
  
  

• pomone

  You can probably shorten the code and make it more "elegant" but all the comparisons will have to be done in one way or the other. For instance, you could scan through all characters and set a flag if you see anything that is a space or a zero, then check the flag once you are done and branch based on whether it is set or cleared. To do something like that, and actually catch some more possible combinations of spaces and zero's, you could do this:

  string s = strNaupaRec.Substring(258,9);

  bool flag = true;

  for (int i = 0; i < s.Length; i++) {

  if (s[ i ] != ' ' && s[ i ] != '0') {

  flag = false;

  }

  }

  if (flag) {

  s = " ";

  } else {

  s = ValidatePostalCode(s);

  }

  I'm not so sure exactly what the original intent of the VB code was though. If it's just to check the zip code and make sure it is valid, then I wouldn't necessarily code it this way. I believe there is a publically available list or database of all valid zip codes and their corresponding states and cities, so I'd download that on a regular bases, put it in a database, and just do a lookup.

  P.S. I haven't figured out how to mark blocks of code yet, so sorry for the spacing, etc.

  HTH,

  Fred

  
  

• kbradl1

  If the task is just to port the code, then just port the code. But it was asked how it could be made more efficient, if there was a better way to do it, and if the code could be shortened. The original code didn't really test for a valid USPS ZIP code anyway, so you may want to try something like this (where s is the string you're checking):
  
  Match m = Regex.Match(s,"^\\s*(\\d{5})(-) (\\d{4}) \\s*$");
  if (! m.Success)
  {
  // non-US
  return "non-US " + s;
  }
  else
  {
  if (m.Groups[1].ToString().CompareTo("005") < 0)
  {
  // non-us
  return "non-US " + s;
  }
  return "Normalized US" + m.Groups[1] + ((m.Groups[3].ToString() == "") "" : "-" + m.Groups[3]);
  }
  
  That's probably the most compact, efficient, "better" way to do it. Strip out all leading and trailing spaces, require 5 digits for the mandatory ZIP, but nothing less than 00500, and allow for a +4 code with or without the dash, but add it in if missing as the USPS requires it. If it doesn't meet those requirements it's not a valid USPS ZIP code and just return the original non-US postal code. You could enhance it my allowing spaces before and after the dash easily if you wanted to.
  
  - Fred
  
  
  

• Fahad Habib

  The thing is, the old code “works” (maybe), so you want to make sure you duplicate the old code before you try to make it “more efficient”. I presume you have some of the old data around to test both versions

   

  Oops - just noticed that the assignment should be

  strData = Validate etc

   

  because

  = is assignment

  == is comparison

   

  sorry about that

  
  
  
• Can this be made more efficient?