Page 15 of 15

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 8:18 am
by Scrumpy.Gums
OK, here goes...haven't done any regex for a while :P No doubt there's something not quite right in there :roll:

Code: Select all

'#^<\?php[ '.PHP_EOL.']'.
'\$arr ?= ?array\([ '.PHP_EOL.']?'.
'(\\'foo\\'|"foo") ?=> ?(\\'bar\\'|"bar")[ '.PHP_EOL.']?'.
'\);$#'
The reason for the (\'foo\'|"foo") bits is so you can't have something like 'foo". There may be a neater way to do this.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 9:01 am
by Chris Corbyn
This is my shot at it.

Code: Select all

 
$fileContentPattern = '~^
  <\?php \s*
  \$arr \s* = \s* array \s* \( \s*
  (["\'])zip\\1 \s* => \s* (["\'])button\\2 \s*
  \);
  ~xs';
 
Our highlighter hates escaping backslashes for some reason :(

PS: I wasn't worried about creating a little manual test file to produce the regex. I'd rather have some peace of mind that I've written a robust regex so I just made a little "scrap" file while I produced it. It can probably be done better. The "x" modifier is extremely useful for making the pattern a bit more readable. The \\1 and \\2 ensure that the closing quotes match the opening one.

Code: Select all

<?php
 
$fileContentPattern = '~^
  <\?php \s*
  \$arr \s* = \s* array \s* \( \s*
  (["\'])zip\\1 \s* => \s* (["\'])button\\2 \s*
  \);
  ~xs';
 
$arr = '<?php $arr = array("zip" => "button");';
 
var_dump(preg_match($fileContentPattern, $arr));

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 9:09 am
by Scrumpy.Gums
Yes, that's much nicer :) The \\1 and \\2 are pretty neat.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 9:10 am
by matthijs
But how can you apply this to the ArrayStorageModel? I mean, now it's "zip" => "button", but next time it's something else. You'd need a general way of making sure you include something parsable, isn't it?

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Thu May 01, 2008 5:08 pm
by Chris Corbyn
matthijs wrote:But how can you apply this to the ArrayStorageModel? I mean, now it's "zip" => "button", but next time it's something else. You'd need a general way of making sure you include something parsable, isn't it?
Sorry, I should have been clearer. This is only for the test case. It's not going inside the class itself :)

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri May 02, 2008 4:04 am
by georgeoc
Just jumping in here - might it be worth saving a serialized array rather than plain var_export() style? Then you could test that the class is producing the correct result using get_file_contents() and a boolean test for unserialize().

Of course you add some overheads doing this.

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri May 02, 2008 5:59 am
by matthijs
Chris Corbyn wrote:
matthijs wrote:But how can you apply this to the ArrayStorageModel? I mean, now it's "zip" => "button", but next time it's something else. You'd need a general way of making sure you include something parsable, isn't it?
Sorry, I should have been clearer. This is only for the test case. It's not going inside the class itself :)
But if it's so specific to this test case, why bother with the (complicated) regex and not just use the exact characters to compare?

Another problem I still see: how is this going to prevent a real-world problem case, in which a file is loaded which contains non-parsable code? Or is there no way to prevent that?

Last, shouldn't we also test for readability of the file in the load method?

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Fri May 02, 2008 8:11 am
by arjan.top
we won't include the file it will just be checked with file_get_contents() and regex

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Sat May 03, 2008 2:34 am
by Chris Corbyn
The real world syntax error case would be down to user error. There's not much we can do in terms of checking before load() is called... if the user has put a syntax error in the file then not much we can do. What we care about here is that the StorageModel itself doesn't generate code with syntax errors.

This was perhaps a bad choice of libraries to write because opinions on how best to test it will vary wildly and cause confusion. Just think about what we're testing now... we're testing that save() produces a valid array... not that load() can detect an invalid array.

I'm just about to head out for a birthday meal and drinks but will finish this class off over the weekend :)

Re: TDD workshop. Anybody welcome; Data storage abstraction.

Posted: Sat May 03, 2008 3:14 am
by matthijs
Ok, that's clear. Let's not dive too deep in the details and issues of this specific implementation then. I'd rather see we make progress with the testing process. If we could test a few more methods and maybe another class or so it'd be great.

If there are difficult issues that take more time and (advanced) discussion, it's fine with me if you just shortly mention them.

I'll be online the next days on and off, after that I'll be away for a week and won't be able to post much.