Re^2: Refactoring challenge.

Thanks Hugo, that helps a bit more :)

A small point: that s/// is pretty inefficient,

Unfortunately, $string =~ s[^.{$pos}\s*][]; doesn't just remove that part of the string that has been printed, it also removes any leading whitespace from the remainder of the string that otherwise would mess up the indenting.

I've been trying to remove the need for that, by having f1() and f2() advance the pointer(s) beyond the whitespace after they locate their relevant positions, so that it gets output on the end of the previous line of output where it does no harm, but so far, I've failed.

However, incorporating it into the _print/_consume sub is a good idea.

And passing the $depth through simplifies things a bit more.

Unfortunately, I cannot flatten the nesting in quite the way you have as the $pos ($length as you have it) will also be undefined if $first is undefined. I realise that this was implicit rather than explicit in the OP.

It's a shame, and something I will attempt to correct if possible, because as you have it, the parser becomes a state machine which would be very nice.

For now, the magic problem doesn't arise as $str is just a string (generated wholey within the outer layers of my code and will not have any magic attached), but it is something that I wasn't aware of and worth noting for the future.

As it stands, the combined refactoring, in-situ of it's surrounding context, has introduced a minor edge case bug that wasn't there before, but the reduction in clutter should make it easier to resolve.

The point I had been at for over a week, was that I had arrived at code that worked, by hacking at my initial attempt, but that it was so fragile that every time I tried to clean it up, it broke badly. By isolating this part from the rest and getting other eyes to look at it, it has simplified the overall thing to the point where it is much less fragile.

Examine what is said, not who speaks.

Silence betokens consent.

Love the truth but pardon error.

Comment on Re^2: Refactoring challenge. Select or Download Code

Replies are listed 'Best First'.
Re^3: Refactoring challenge. by hv (Prior) on Mar 06, 2005 at 13:28 UTC
Unfortunately, [the substitution] also removes any leading whitespace from the remainder of the string Oops, I completely missed that. But adding a trailing `$_[0] =~ s/^\s+//;` [download] to `$_consume()` is enough to fix that. Unfortunately, I cannot flatten the nesting in quite the way you have as the `$pos` (`$length` as you have it) will also be undefined if `$first` is undefined. Both your original code and dragonchild's refactor contradict this - both use the returned `$pos1` (`$pos`) unaltered when `$first` is undefined. For now, the magic problem doesn't arise as `$str` is just a string (generated wholey within the outer layers of my code and will not have any magic attached) As I remember it, the principle (and most embarrassing) time that it becomes a problem is when the parameter is tainted. Hugo	[reply] [d/l] [select]
Re^4: Refactoring challenge. by BrowserUk (Patriarch) on Mar 06, 2005 at 14:08 UTC
both use the returned $pos1 ($pos) unaltered when $first is undefined. Ah! (blush} I oversimplified. In-situ, the call to f1() is followed by: `return unless defined $pos;` [download] Information which makes your reduction very pertinent and holds out the possibility of reducing the whole snippet to a single level state machine. The block of code in the OP actually sits within a while loop in the real code. What the code does, is takes a string of the form: `{ a => [ SCALAR(0x18bb45c), { a => b, c => d, e => f, g => h, }, [ 1, + 2, 3,` [download] and pretty prints it like this: `{ a => [ SCALAR(0x18bf6a4), { a => b, c => d, e => f, g => h, }, [ 1, 2, 3, 4, 5, 6,` [download] or like this `{ a => [ SCALAR(0x18bf6a4), { a => b, c => d, e => f, g => h, }, [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ],` [download] or this `{ a => [ SCALAR(0x18bf6a4), { a => b, c => d, e => f, g => h, }, [ +1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ], {` [download] depending upon a `width` argument supplied--but there is an additional twist. The code generating the input does it in chunks as it recursively traverses an unknown datastructure. The pretty printer gets fed those chunks (it is called from the STORE routine of a tied scalar), and is charged with pretty printing the (accumulated) string each time it goes over the `width` limit without waiting for the whole string, or even a complete set of balanced text to accumulate. The idea is to avoid having to hold the whole of the potentially enormouse string in memory. I probably should have posted the whole thing for communal refactoring, but it has so many dependancies that it would be asking a lot for anyone to look at it. Not to mention, that I was embarassed by the code in every respect--except that it worked! Examine what is said, not who speaks. Silence betokens consent. Love the truth but pardon error.	[reply] [d/l] [select]