in reply to Re^2: patchable settings
in thread patchable settings

I don't understand why people are reluctant to use techniques like version codes here. (I say this because ive noticed others also dislike version codes.) To me a settings field is just a data format which is really a file format. And its usually better to code version strings into such systems early while its easy. The flexibility such a mechanism offers in a situation like here is quite nice if you ever have reason to change the way things work. And I've had call to use it elsewhere on PM to good effect already.

# note that these two vars go together to a certain extent. my $magic="===&01"; my $escape="%=&"; sub unpack_hash { my ($str)=shift; my %hash; if (substr($str,0,6) eq $magic) { substr($str,0,6)=''; %hash=map { map { s/%(\d)/substr($escape,$1,1)/ge; $_ } split /=/,$_ } split /&/,$str; } else { # old getVars() support... } } sub pack_hash { my $hash=shift; return $magic.join("&",map { join "=",map { (my $v=$_)=~s/([$escape])/'%'.index($escape,$1)/ge $v } $_ => $hash{$_} } sort keys %$hash); }

Since bullet proofing getVars() probably means it faster to do than the current method, I personally dont see why we shouldn't do it.


---
demerphq

    First they ignore you, then they laugh at you, then they fight you, then you win.
    -- Gandhi

    Flux8


• Update:  
Note that only a sliver of extra code would allow using Data::Dumper for the values if requested to do so (with only a tiny overhead were the hash values reference free). Ultimately that would be where I would go. Encode the keys with extra info when packed to indicate their value needs thawing. Anyway.... :-)


Replies are listed 'Best First'.
Re^4: patchable settings
by ysth (Canon) on Sep 28, 2004 at 07:42 UTC
    My reluctance is based on two things, both very minor; first, it means taking really short simple code and making it less so; second, it reduces the readability of the raw data.

    It's your call; if you say so, I'll take what you have above and integrate it with my unpackVars/packVars (though I'd much prefer just to use the escape() and unescape() that are already there).

      it means taking really short simple code and making it less so;

      Except that less code is actually executed and there is less stack overhead.

      second, it reduces the readability of the raw data.

      How does it do that? The current mechanism uses url encoding to handle \W chars. This mechanism would leave everything alone except for =,& and %. (Pick three less common chars if you want... I did in the personal nodelet stuff.)

      Im not saying you should definatively do anything, im just arguing that my suggestion is sound. Im hoping others pipe up with their thoughts too (although really only people that use setting's will have any experience to guide their thoughts). My position is that making my proposed changes would make a lot of things more robust and easy to deal with and actually be a marginaly net gain in efficiency due to less code and faster encoding. I dont see why we would give up those benefits for what we have now given the relative costs.


      ---
      demerphq

        First they ignore you, then they laugh at you, then they fight you, then you win.
        -- Gandhi

        Flux8


        I've put our cb conversation (unedited, so there's surrounding cruft) here for the nonce.
Re^4: patchable settings
by ysth (Canon) on Oct 05, 2004 at 19:03 UTC
    How does this look (patch is on top of [id://patchable settings;displaycode|previous patch]):
    --- Everything.pm 2004-10-05 11:28:38.716515200 -0700 +++ Everything.pm.new 2004-10-05 11:31:58.093204800 -0700 @@ -261,13 +261,28 @@ sub unescape sub unpackVars { my ($vars) = @_; + my $format_version = "00"; + + # version 00: original format + # version 01: keys are escaped, not just values + $format_version = $1 if $vars =~ s/^==(\d\d)&//; return {} unless $vars; - my %vars = map { split /=/ } split (/&/, $vars); - foreach (keys %vars) { - unescape $vars{$_}; - if ($vars{$_} eq ' ') { $vars{$_} = ""; } + my %vars; + + for (split /&/, $vars) { + my ($k,$v) = split /=/; + + if ($format_version > 0) { + unescape( $k, $v ); + } else { + unescape( $v ); + } + + $v = '' if $v eq ' '; + + $vars{$k} = $v; } return \%vars; @@ -296,7 +311,9 @@ sub packVars $$varsref{$_} = " " unless $$varsref{$_}; } - return join("&", map( $_."=".escape($$varsref{$_}), keys %$varsre +f) ); + # current format version: 01 + return join("&", "==01", + map( escape($_)."=".escape($$varsref{$_}), keys %$varsref) ); } ##################################################################### +########
    I think I've placed more emphasis (or at least tried to) on readability and reuse than what you had suggested. I'm guessing it will be about as fast as the existing code. If you are ok with this, I'll try it out on the test server.

      I think I've placed more emphasis (or at least tried to) on readability and reuse than what you had suggested.

      Yeah style wise I agree. But there was a point to my use of that type of packing scheme. Its a lot faster and the resulting text is smaller than the current one and it renders the text less illegibile in raw form. Just avoiding the overhead of the escape call is worthy IMO. Also the scheme I used can be made a litte more dynamic and self documenting by a more verbose rewrite. I think its worth finding out the full requirements of this (ie possible charset restrictions) and getting a fast light implementation done once and for all. Settings are used a _lot_, shaving time on them is very worthwhile IMO.

      But i definately like this a lot more than the earlier ideas. One last thing, can I suggest that you use == at the start of the string? What you have there is a possible valid start of a vars. '=00&' could come from { ''=>'00','1'=>'1' } for instance. I suggest '==00&' so that its impossible to occur under the current codebase.


      ---
      demerphq

        First they ignore you, then they laugh at you, then they fight you, then you win.
        -- Gandhi

        Flux8


        Changed to use ==.

        Even the 100+ sub calls it is doing per page is not going to take that long, but I'll take a stab at improving it later today or Sunday.