• DonaldK (unregistered)

    All the world's a string.

    Or a SubString... at the very least.

  • java.lang.Chris; (cs)

    Sometimes a regexp really is the right answer ...

  • @Deprecated (cs)

    Wow, so many inefficiencies I don't even know where to start!

    • Rebuilds array every time it is called
    • Uses an array instead of c >= '0' and c <= '9'
    • Calls substring repeatedly in inner loops
    • Does not exit early from outer for loop
    • Using strings instead of booleans

    Ugh. Maybe the array is used in case sometime in the future, a new number is added between '4' and '5'.

  • trtrwtf (unregistered)
    Martin continues, "I'm not sure which is worse, the string return value of '0' or '1' or the ArrayList of numbers. I've found plenty of other similar functions but this is a nice standalone example.

    Ooh! Ooh! Pick me, I know!

    Is it when they use strings as booleans?

    (I'm thinking the use of the .Add() method and the .Length field has to be a simply [s]typo[/s]Microsoft being helpful... picking on them for that would be shooting ducks in a barrel)

  • @Deprecated (cs) in reply to @Deprecated
    @Deprecated:
    Wow, so many inefficiencies I don't even know where to start!
    • Rebuilds array every time it is called
    • Uses an array instead of c >= '0' and c <= '9'
    • Calls substring repeatedly in inner loops
    • Does not exit early from outer for loop
    • Using strings instead of booleans

    Ugh. Maybe the array is used in case sometime in the future, a new number is added between '4' and '5'.

    Wait, WTF??? bool validChars = true; and then 4 lines later: string matchedChar = "N";

  • emaN ruoY (unregistered)

    Somebody has a very confused, multipersonal mind. First, as stated, sometimes RegEx really is the answer. Second, there are too many different styles in the code. As noted, using bool and string in the same method to indicate true/false.

    I half expect that the Caller probably sets a string variable and then through tedious manipulation turns it back into a boolean value.

  • airdrik (cs)

    [s]There is no String, only Bool[/s]There is no Bool, only String

  • RaceProUK (cs) in reply to emaN ruoY
    emaN ruoY:
    First, as stated, sometimes RegEx really is the answer.
    Assuming the code is .NET, I think Int32.TryParse would be quicker. The method then becomes:
    private Boolean checkChars(string destination)
    {
        Int32 sink;
        return Int32.TryParse(destination, out sink);
    }
    
    Or, to keep the signature intact:
    private String checkChars(string destination)
    {
        Int32 sink;
        return Int32.TryParse(destination, out sink) ? "1" : "0";
    }
    
  • Alex (unregistered) in reply to java.lang.Chris;

    I think a loop with said "c >= '0' and c <= '9'" would beat the RegExp performance-wise.

    foreach (char c in input)
    {
       if ((c < '0')="" ||="" (c=""> '9'))
       {
          LogDebug("Invalid character found")
          return false;
       }
    }
    LogDebug("Everything purrrrfect")
    return true;
    
  • dw (unregistered)

    Maybe this function was originally a system call.

    That would explain the return value of "0" on success and "1" on failure.

  • Alex (unregistered) in reply to RaceProUK

    This assumes that the string fits into a 32 bit signed integer (–2147483648 to 2147483647). An international phone number or some kind of account number could already exceed that limit.

  • MB (unregistered) in reply to RaceProUK

    Surely int.parse will limit valid results to under 2147483647

  • TGV (cs)

    Hey, you know student code is not a WTF! And this can't have been written by anyone with more than a week of programming experience. Right? Please?

    Or do you hire linguistic majors for your programming?

  • TGV (cs) in reply to dw
    dw:
    Maybe this function was originally a system call.

    That would explain the return value of "0" on success and "1" on failure.

    Which system would that be???

  • dw (unregistered) in reply to TGV

    I meant "system call" in the sense of a call to an outside process like a BASH script. Sorry -- haven't been fully caffeinated yet.

  • Steve The Cynic (cs) in reply to RaceProUK
    RaceProUK:
    emaN ruoY:
    First, as stated, sometimes RegEx really is the answer.
    Assuming the code is .NET, I think Int32.TryParse would be quicker.
    Nice try, but you need to exclude strings beginning with "-" in some way, as TryParse would presumably say that e.g. "-456" is valid...
  • Watson (cs) in reply to @Deprecated
    @Deprecated:
    @Deprecated:
    Wow, so many inefficiencies I don't even know where to start!
    • Rebuilds array every time it is called
    • Uses an array instead of c >= '0' and c <= '9'
    • Calls substring repeatedly in inner loops
    • Does not exit early from outer for loop
    • Using strings instead of booleans

    Ugh. Maybe the array is used in case sometime in the future, a new number is added between '4' and '5'.

    Wait, WTF??? bool validChars = true; and then 4 lines later: string matchedChar = "N";

    Yup; sorry to break it to you but there is something worse than using strings instead of booleans....
  • B. D. Johnson (unregistered) in reply to @Deprecated

    Um, I think you'r forgetting about the "11" in 911.

  • ahydra (unregistered)

    "Ugh. Maybe the array is used in case sometime in the future, a new number is added between '4' and '5'."

    I think I'll change my phone number to 07709 34724½8¾. No way telemarketers will find me then!

  • Zylon (cs)

    WTF is "Sting Verification"?

  • Jamie (unregistered) in reply to @Deprecated

    My 2 year old seems to thing 6 comes between 4 and 5...

  • LabSpecimen (unregistered)

    We had fun with this one at work. Here's my version:

    private boolean checkChars(string destination) { int cnt = 0; if (destination != null) { for (int i = 0; i < destination.length(); i++) { (destination.charAt(i) == '0') ? cnt++ : (destination.charAt(i) == '1') ? cnt++ : (destination.charAt(i) == '2') ? cnt++ : (destination.charAt(i) == '3') ? cnt++ : (destination.charAt(i) == '4') ? cnt++ : (destination.charAt(i) == '5') ? cnt++ : (destination.charAt(i) == '6') ? cnt++ : (destination.charAt(i) == '7') ? cnt++ : (destination.charAt(i) == '8') ? cnt++ : (destination.charAt(i) == '9') ? cnt++ : cnt = cnt; } } return (destination != null && cnt == destination.length()) ? true : false; }

  • C-Derb (unregistered) in reply to @Deprecated
    @Deprecated:
    Wow, so many inefficiencies I don't even know where to start! ... - Does not exit early from outer for loop ...
    If the code exited from the outer loop as soon as the first invalid character was found, then the debug log would not give a full report of ALL invalid characters in the destination string. Clearly a verbose and accurate debug log is worth the price of completing the outer loop, um, right?
  • Harrow (unregistered) in reply to @Deprecated
    @Deprecated:
    Wow, so many inefficiencies I don't even know where to start!
    • Rebuilds array every time it is called
    • Uses an array instead of c >= '0' and c <= '9'
    • Calls substring repeatedly in inner loops
    • Does not exit early from outer for loop
    • Using strings instead of booleans

    Ugh. Maybe the array is used in case sometime in the future, a new number is added between '4' and '5'.

    The Nobel Prize in Mathematics has been awarded to a California professor who has discovered a new number. The number is "bleen" which he claims belongs between six and seven. --George Carlin, The New News

    -Harrow.

  • redmasq (unregistered) in reply to @Deprecated

    If only the validity of characters I would do something like:

    using System.Text.RegularExpressions;

    public class Blah { private static readonly Regex CharCheck = new Regex("[^0-9]"); private bool CheckCharacters(string destination) { return !(CharCheck.Match(destination).Success); } }

    Syntax looked C#'ish, so wrote in that syntax, but similar could be done in Java.

    Making it static means only one instance of the regex running. If used a lot, I'd probably add the compiled flag. In practice I haven't seen any threading issues with doing similar, but I would double check for a critical application since only the static methods are guaranteed thread-safe (Match is used as an instance method in this case). The readonly flag should help the compiler with optimizations.

    If the logging is needed, just modify to keep the value of success, and use the Matches method instead: iterate thru the returned collection, logging as appropriate.

    Just my two-cents.

  • Zmee (unregistered)

    Just wow! And replacing this with a single line:

    return java.util.regex.Pattern.matches("^\\d*$", destination);

    (And yes, it is d* rather than d+; there is no requirement for the string to be non-empty.)

  • bene (unregistered) in reply to airdrik
    airdrik:
    [s]There is no String, only Bool[/s]
    You surely mean "Zuul"?
  • area driver (unregistered) in reply to @Deprecated
    @Deprecated:
    Ugh. Maybe the array is used in case sometime in the future, a new number is added between '4' and '5'.

    Hi Depricated, are you free for lunch on the Fouivth of Smarch?

  • foo (unregistered) in reply to emaN ruoY
    emaN ruoY:
    Somebody has a very confused, multipersonal mind. First, as stated, sometimes RegEx really is the answer. Second, there are too many different styles in the code. As noted, using bool and string in the same method to indicate true/false.
    And even two different string Booleans (N/Y and 0/1).

    (Yeah, and none of them includes FILE_NOT_FOUND, blah.)

  • C-Derb (unregistered)

    Mmmm...I love me some recursion:

    private bool ContainsChars(string destination)
    {
      if (string.IsNullOrEmpty(destination)) return false;
      int i;
      return int.TryParse(destination.Substring(0, 1), out i) ? ContainsChars(destination.Substring(1)) : true;
    }
    
  • pauly (cs)

    This looks like pre-optimization code. You write functions like this so you can rewrite them in the future when the boss demands the code runs faster and uses less memory. This is more clever than using some sort of wait function, as it really does increase the CPU rather than artificially slowing it down.

  • Matt (unregistered)

    return Regex.Match("[\d]", destination).Success;

  • Nappy (unregistered)

    Written by the guy who gets payed per log line checked?

  • M (unregistered)

    C# with extension method and lambda:

        private static bool ContainsNonDigits(this string destination)
        {
            return destination.Any(c => c < '1' || c > '9');
        }
    

    Example:

    string foo = "abc123"; if (foo.ContainsNonDigits()) { Blah, blah, blah... }

  • Kristopher Ives (unregistered)

    A man once mad regular expressions. That apparently wasn't this man. Honestly dropping this in my list of "stuff you do without regex"

  • M (unregistered) in reply to M
    M:
    C# with extension method and lambda:
        private static bool ContainsNonDigits(this string destination)
        {
            return destination.Any(c => c < '1' || c > '9');
        }
    

    Example:

    string foo = "abc123"; if (foo.ContainsNonDigits()) { Blah, blah, blah... }

    ... or public if you want to use it outside the class it's defined in. :)

  • Nagesh (cs) in reply to LabSpecimen
    LabSpecimen:
    We had fun with this one at work. Here's my version:

    private boolean checkChars(string destination) { int cnt = 0; if (destination != null) { for (int i = 0; i < destination.length(); i++) { (destination.charAt(i) == '0') ? cnt++ : (destination.charAt(i) == '1') ? cnt++ : (destination.charAt(i) == '2') ? cnt++ : (destination.charAt(i) == '3') ? cnt++ : (destination.charAt(i) == '4') ? cnt++ : (destination.charAt(i) == '5') ? cnt++ : (destination.charAt(i) == '6') ? cnt++ : (destination.charAt(i) == '7') ? cnt++ : (destination.charAt(i) == '8') ? cnt++ : (destination.charAt(i) == '9') ? cnt++ : cnt = cnt; } } return (destination != null && cnt == destination.length()) ? true : false; }

    Not much work at your work, I see.

  • oops (unregistered) in reply to M
    M:
    C# with extension method and lambda:
        private static bool ContainsNonDigits(this string destination)
        {
            return destination.Any(c => c < '1' || c > '9');
        }
    

    Example:

    string foo = "abc123"; if (foo.ContainsNonDigits()) { Blah, blah, blah... }

    Better to do: return destination.Any(c => !char.IsDigit(c));

    IsDigit never forgets that 0 is a digit too, even though it is also < '1'.

  • Octavian (unregistered) in reply to oops
    oops:
    M:
    C# with extension method and lambda:
        private static bool ContainsNonDigits(this string destination)
        {
            return destination.Any(c => c < '1' || c > '9');
        }
    

    Example:

    string foo = "abc123"; if (foo.ContainsNonDigits()) { Blah, blah, blah... }

    Better to do: return destination.Any(c => !char.IsDigit(c));

    IsDigit never forgets that 0 is a digit too, even though it is also < '1'.

    But I want to call it with Ⅷ.

  • M (unregistered)

    i did some actual testing of this one (in Java, see code below) and the results really speak for themselves (each method called 2M times):

    Enterprisey: 19858 simple: 192

    import java.util.ArrayList;
    
    public class Test {
    
        public static void main(String[] args) {
    	String s = "193485923905820394029850298402983409284032840238402984";
    	String t = "34534534503985039845038450394850394850398503498503985a";
    	long l = System.currentTimeMillis();
    	for (int i = 0; i < 1000000;="" i++)="" {="" checkchars(s);="" checkchars(t);="" }="" l="System.currentTimeMillis()" -="" l;="" system.out.println("enterprisey:="" "="" +="" l);="" l="System.currentTimeMillis();" for="" (int="" i="0;" i="">< 1000000;="" i++)="" {="" checkchars2(s);="" checkchars2(t);="" }="" l="System.currentTimeMillis()" -="" l;="" system.out.println("simple:="" "="" +="" l);="" }="" private="" static="" boolean="" checkchars2(string="" in)="" {="" for="" (char="" c="" :="" in.tochararray())="" {="" if="" (!(c="">= '0' && c <= '9'))="" {="" return="" false;="" }="" }="" return="" true;="" }="" private="" static="" string="" checkchars(string="" destination)="" {=""></=><string> validCharacters = new ArrayList<string>();
    
    	validCharacters.add("0");
    	validCharacters.add("1");
    	validCharacters.add("2");
    	validCharacters.add("3");
    	validCharacters.add("4");
    	validCharacters.add("5");
    	validCharacters.add("6");
    	validCharacters.add("7");
    	validCharacters.add("8");
    	validCharacters.add("9");
    
    	boolean validChars = true;
    
    	for (int i = 0; i < destination.length();="" i++)="" {="" string="" matchedchar="N" ;="" for="" (string="" character="" :="" validcharacters)="" {="" if="" (destination.substring(i,="" i="" +="" 1)="=" character)="" {="" matchedchar="Y" ;="" break;="" }="" }="" if="" (matchedchar="=" "n")="" {="" logdebug("checkchars="" -=""> Invalid character found: " + destination.substring(i, i + 1));
    
    		validChars = false;
    	    }
    	}
    
    	String charStatus;
    
    	if (validChars) {
    	    LogDebug("checkChars -> All characters ok");
    
    	    charStatus = "0";
    	} else {
    	    LogDebug("checkChars -> Invalid characters found");
    
    	    charStatus = "1";
    	}
    
    	return charStatus;
        }
    
        private static void LogDebug(String string) {
        }
    
    }
    </string></string>
  • Ello (unregistered)

    Why not ask Zoidberg and usw

    if (Regex.IsMatch(input, "^[0-9 ]+$")) return true;

  • Illuminati (unregistered) in reply to @Deprecated

    Its not between 4 and 5 it's between 3 and 4 and its called bleem:

    http://www.strangehorizons.com/2000/20001120/secret_number.shtml

    Now that you know they'll try and kill you to keep it secret!

  • Jimmy Carter (unregistered) in reply to M
    M:
    i did some actual testing of this one (in Java, see code below) and the results really speak for themselves (each method called 2M times):

    Enterprisey: 19858 simple: 192

    import java.util.ArrayList;
    
    [snip]
    
    

    You left out the logging. Logging was clearly specified in the requirements document.

  • Speakerphone Dude (cs) in reply to @Deprecated
    @Deprecated:
    Maybe the array is used in case sometime in the future, a new number is added between '4' and '5'.

    This could also be hugely convenient at some point in the future if 7 8 9!

  • R (unregistered) in reply to @Deprecated

    Or just regex /\d+/

  • Zylon (cs) in reply to bene
    bene:
    airdrik:
    [s]There is no String, only Bool[/s]
    You surely mean "Zuul"?
    You are dumb.
  • Aargle Zymurgy (unregistered)

    Years ago when I started teaching, I already knew programmers would just code the very first thing that occurred to them to solve a problem. I think I had a small measure of success when I told them "If you can't think of at least two ways to do what you're trying to do, you're not actually thinking about the problem. Sure, you might think of some really bad way to solve the problem, but at least you've got something to compare your other idea to. You can improve from there."

  • operagost (cs) in reply to TGV
    TGV:
    dw:
    Maybe this function was originally a system call.

    That would explain the return value of "0" on success and "1" on failure.

    Which system would that be???
    An embedded system with no storage space, of course.

  • Linux Pro (unregistered)

    I fear that performance-wise it doesn't even matter.

  • Wetlands (unregistered)

    Add logging and change the return type if you need to but the basic functionality is:

    return s.ToCharArray().Any(c => "1234567890".Contains(c) == false);

Leave a comment on “Stringy Sting Verification”

Log In or post as a guest

Replying to comment #:

« Return to Article