[Prophet] A few random questions on the source

[Gianni Ceccarelli]

On 2008-08-19 Shawn M Moore <sartak at bestpractical.com> wrote:
> Thanks for all these good (and sometimes (rightfully) harsh) comments!

Hmmm. I didn't actually intend for any of them to be harsh, they may
be seem that way because they are just short comments… I'll try to be
a bit more verbose, so as not to sound curt/harsh.

> > Index: lib/Prophet/CLI.pm
> > ===================================================================
> > --- lib/Prophet/CLI.pm	(revision 15237)
> > +++ lib/Prophet/CLI.pm	(working copy)
> > @@ -28,6 +28,7 @@
> >      lazy    => 1,
> >      handles => [qw/handle resdb_handle config/],
> >      default => sub {
> > +        # Prophet::App::require($pack_name) returns without doing
> > anything: what's this call for?
> 
> require just loads the class up. Perl's own require builtin has a
> deficiency: Perl doesn't understand that require "Foo::Bar" should
> load Foo/Bar.pm. So we have a number of workarounds to get the same
> semantics, this require method being one of them.

Yes, I've read the method (although the usual idiom is «eval "require
$package_name"», it has its own share of problems)

> Prophet::App::require is actually supposed to be invoked as a method
> (so, Prophet::App->require($pack_name)). If you were poking at this
> manually that's why it would appear to do nothing.

Yes, it's clearly intended to be called that way, but Prophet::CLI
calls it as «Prophet::App->require;», which lacks the second
parameter, thus exiting without actually requiring anything. Or, more
probably, I'm missing something obvious.

> > Index: lib/Prophet/Record.pm
> > ===================================================================
> > --- lib/Prophet/Record.pm	(revision 15237)
> > +++ lib/Prophet/Record.pm	(working copy)
> > @@ -114,7 +114,7 @@
> >              @args
> >          );
> >      } elsif ( $foreign_class->isa('Prophet::Record') ) {
> > -
> > +# uh? nothing happens?
> 
> This elsif clause avoids the exception below.

Yes, my question was poorly stated. Let me rephrase: what should
happen when I
«My::Record->register_reference('stuff',My::Other::Record)»?
Currently, nothing seem to happen, not even a warning. Either the call
is meaningless, or I'm missing the implementation.

> > Index: lib/Prophet/Replica.pm
> > ===================================================================
> > --- lib/Prophet/Replica.pm	(revision 15237)
> > +++ lib/Prophet/Replica.pm	(working copy)
> > @@ -127,6 +127,8 @@
> >      my $self = shift;
> >      my $url  = shift;
> >  
> > +    # shouldn't that be split /:/, $url,2 ?
> > +    # just in case $url contains more than one colon
> 
> Perl is smart enough to handle this correctly when $url contains
> multiple colons. For split (and only for split!) it looks at how many
> variables you're sticking the result into and does what you want.

From perlfunc:

> When assigning to a list, if LIMIT is omitted, or zero, Perl
> supplies a LIMIT one larger than the number of variables in the
> list, to avoid unnecessary work.

Note the "one larger". «my ($schema,$url)=split
/:/,'prophet:file:///tmp/stuff'» puts 'prophet' in $schema and 'file'
in $url.

> > +    # uh? weird way of boolifying
> >      my $inside_edit = $state_handle->current_edit ? 1 : 0;
> 
> I don't quite see how this is weird. Though it is perhaps
> unnecessary.

Right, "unnecessary" is a much better term for what I meant.

> Changesets are not directly cumulative, only indirectly. Changesets
> are _ordered_ so that if you're about to apply changeset 5, you will
> have already applied changeset 4.

Uhm. What happens if we get cs 5 before getting cs 4? Or, where is the
check to make sure this does not happen? (I am a bit lost in the
changeset handling logic)

> > --- lib/Prophet/Resolver/FromResolutionDB.pm	(revision
> > 15237) +++ lib/Prophet/Resolver/FromResolutionDB.pm	(working
> > copy) @@ -27,6 +27,7 @@ $answer_map{$key} ||= $answer;
> >          $answer_count{$key}++;
> >      }
> > +    # this is random in case of a tie
> 
> Yeah, we should probably prompt in case of a tie. This seems to do
> fine with picking an arbitrary winner though.

Oh well, "works fine" is justification enough.

> Thanks a lot for all of your comments, Gianni. Tonight I'll go through
> this mail I just wrote and clean up the code.

Glad to be of service :)

-- 
	Dakkar - <Mobilis in mobile>
	GPG public key fingerprint = A071 E618 DD2C 5901 9574
	                             6FE2 40EA 9883 7519 3F88
	                    key id = 0x75193F88

If the girl you love moves in with another guy once, it's more than
enough. Twice, it's much too much.  Three times, it's the story of your
life.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
Url : http://lists.bestpractical.com/pipermail/prophet/attachments/20080819/4fd71349/attachment.pgp