Check binary string, running into errors here

Question

I'm using this function to check if binary is correct, I know it looks sloppy.. I'm not sure how to write the function that well.. but it doesn't seem to work!

If binary = 10001000 it says malformed, even though it's not.. what is wrong in my function?..

function checkbinary($bin) {
    $binary = $bin;
    if(!strlen($binary) % 8 == 0){
        return 1;
    }
    if (strlen($binary) > 100) { 
        return 1;
    } 
    if (!preg_match('#^[01]+$#', $binary)){ //Tried without !
        return 1;
    } 
    if (!is_numeric($binary)) {
        return 1;   
    }
}

if (checkbinary("10001000") != 1) {
  echo "Correct";
} else {
  echo "Binary incorrect";
}

Why does this function always say 10001000 is incorrect?

More of a suggestion for the future than an answer in the present, but if you're checking multiple conditions like this, you may wish to return descriptive messages for each check, like "$binary is not numeric" for the is_numeric test, and then return something like "Correct" and the end of the function, if none of the other returns executed. This helps you identify which check is not behaving as you'd like. On a side note, I think your regex requirement of the string containing only 1s and 0s makes the is_numeric check unnecessary. — Lazy Bob
– Lazy Bob, Commented Jan 30, 2010 at 12:20
@Lazy Bob: I think he doesn't understand the concept of return very well, the strlen($binary) > 100 check makes me awfully suspicious he's trying to do a completely different thing. — Alix Axel
– Alix Axel, Commented Jan 30, 2010 at 12:24
A return value of "1" means error in the "original" implementation (see test case). Whether those tests make sense or not is hard or virtually impossible to decide for us (given the small amount of information we have). My only complain about it right now is that the function does more than the name implies (...and maybe the strange choice of return values =)) — VolkerK
– VolkerK, Commented Jan 30, 2010 at 12:33
@mememememe: Do you want to consider 01234567890123456789 ... {104 chars} for instance as a valid binary number? — Alix Axel
– Alix Axel, Commented Jan 30, 2010 at 12:58

VolkerK · Accepted Answer · 2010-01-30 12:27:31Z

4

if(!strlen($binary) % 8 == 0){ should be

if( strlen($binary) % 8 !== 0 ){

edit and btw: Since you're already using preg_match() you can simplify/shorten the function to

function checkbinary($binary) {
  return 1===preg_match('#^(?:[01]{8}){0,12}$#', $binary);
}

This allows 0 - 12 groups of 8 0/1 characters which includes all the tests you currently have in your function:

strlen()%8 is covered by {8} in the inner group
strlen() > 100 is covered by {0,12} since any string longer than 8*12=96 characters would trigger either the first if or the >100 test
0/1 test is obvious
is_numeric is kinda superfluous

edit2: The name checkbinary might not be a perfect choice for the function. I wouldn't necessarily expect it to check for 8bit/byte alignment and strlen()<=100.

edited Jan 30, 2010 at 12:27

answered Jan 30, 2010 at 12:00

VolkerK

96.3k20 gold badges169 silver badges232 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Alix Axel Over a year ago

preg_match() either returns 0 or 1 so the 0!== is unnecessary.

VolkerK Over a year ago

@Alix: I wanted it to return true/false and since I don't like the type juggling for boolean types I've added an explicit test. (That's one thing I like about C#)

Alix Axel Over a year ago

I see, return (bool) preg_match(...); would be more explicit though.

oni-kun Over a year ago

A thousand thanks, It brought a smile to my face when I saw just one line, You know I should really learn some regex. I can use your explaination to learn from it, you're perfect!

VolkerK Over a year ago

@Alix: Maybe so, matter of choice. I change it to 1===preg_match() since it could also return FALSE in case of an error and 1 is the only return value that means "match", anything else is a "failed".

Alix Axel · Accepted Answer · 2010-01-30 12:19:39Z

0

function checkbinary($bin) {
    return preg_match('#^[01]+$#', $bin);
}

Some tests: http://www.ideone.com/3D9SQetX and http://www.ideone.com/1HCCtxVV.

edited Jan 30, 2010 at 12:19

answered Jan 30, 2010 at 12:05

Alix Axel

155k100 gold badges406 silver badges509 bronze badges

2 Comments

VolkerK Over a year ago

This is only a partial replacement for the "original" function as posted by mememememe.

Alix Axel Over a year ago

@VolkerK: Check my comment on his question, it doesn't make a lot of sense.

John Parker · Accepted Answer · 2010-01-30 12:55:32Z

0

I'm not entirely sure what you're attempting to achieve, but you might be able to get there via the following...

if($binaryString == base_convert($binaryString, 2, 2))...

In essence, this is comparing the results of a conversion from binary to binary - hence if the resultant output is identical, the input must be a valid binary string.

answered Jan 30, 2010 at 12:55

John Parker

54.5k11 gold badges133 silver badges131 bronze badges

1 Comment

Alix Axel Over a year ago

Throws warnings if a non-binary number is provided I believe.

Collectives™ on Stack Overflow

Check binary string, running into errors here

3 Answers 3

5 Comments

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related