Re: duplicated code? An open letter to the Author
Peter Karman
On Jan 18, 3:08 pm, mackler <adammack...@gmail.com> wrote:
> I'm looking at two methods in two modules:
>
> in Rose/HTMLx/Form/Related/RDBO/Metadata.pm:
> show_related_field_using()
> in Rose/DBx/Object/MoreHelpers.pm:
> unique_value()
>
> It looks to me as if there might be some duplication of code here. I
> noticed this since the output from each appears in a least one place,
> and in those at-least-two places it seems reasonable to expect a user
> to want consistent output in all of those places.
It's true that those 2 methods both look for the first char-like,
single unique column defined. But show_related_field_using() returns a
column name and unique_value() returns a column value.
>
> Specifically, Rose::HTMLx::Form::__set_menu_options uses
> show_related_field_using to get the text to display in a form's
> pulldown menus where the choices represent rows in a related table.
> On the other hand, the edit.tt template from the YUI uses
> unique_value() to choose text to display as a label for the row/object
> being currently edited. My guess is that the output of unique_value
> gets used elsewhere as well.
Yes, unique_value() is called in various places, among them
foreign_field_value() in RHxFR.
>
> Maybe I'm missing something, so first let me ask you this: is there a
> suggested optimal way to display a text label for a row of a table
> that does not have a text-based, single-column, unique key? (For
> example if first_name and last_name each have their own column and I
> want the row label to contain both names?)
override unique_value() in the RDBO class.
sub unique_value {
my $self = shift;
return join(' ', $self->first_name, $self->last_name);
}
or better:
sub unique_value {
my $self = shift;
return $self->full_name;
}
sub full_name {
my $self = shift;
return join(' ', $self->first_name, $self->last_name);
}
>
> The first solution I came up with was to copy the init_metadata method
> out of the base Form.pm that Garden made for me, and to override it in
> the specific Form.pm file for the class that has the pulldown menu. I
> then added one line to the method setting related_field_map for that
> specific class, so the whole overriding method looked like:
>
> sub init_metadata {
> my $self = shift;
> return $self->metadata_class->new(
> form => $self,
> controller_prefix => 'RDGC',
> related_field_map => { 'user_id' => 'full_name' },
> );
>
> }
>
> The first problem with this solution is that it's going to get wiped
> out next time I plant() the garden, and since I'm still creating the
> database, I re-plant it every few hours to make sure everything is
> still
> working.
yes, that is a problem. I don't have a good clean way of dealing with
that myself. I have tended to either (a) live with the ugly PKs during
rapid schema devel, (b) keep a copy of monkey-patched code around, re-
plant() as necessary and then copy/paste it back into the proper
classes after the scaffolding is created, or (c) plant once and then
hand-edit classes as my schema changes. I tended to do (c) anyway
after my schema had reaching a sufficient maturity and/or my copy/
paste method got too painful.
>
> The second problem with this is that this is in the class making the
> menu, not in the class referred to by the menu. While this change
> fixed my pulldown menus, as mentioned above the unique_value method is
> not affected by this related_field_map.
>
> So before I do too much coding, it's probably worth my asking you if
> there exists a good way to make one change that will turn all my
> displayed, integer, surrogate primary key values into displayed,
> natural keys, including values from multiple key-columns as necessary?
I think I've just patched Rose::HTMLx::Form::Related to move toward
that goal. Please try it from svn trunk here:
https://svn.msi.umn.edu/sw/perl/
You can see the mod I just made here:
https://trac.msi.umn.edu/trac/sw/changeset/604
>
> If the answer to that question is no, then my next thought is that
> perhaps the show_related_field_using and unique_value should be
> combined somehow for consistency? Perhaps show_related_field_using
> could itself call unique_value so that changing the latter will fix
> the former?
yes, that is essentially what I did above.
let me know how it works for you and I'll push a 0.19 to CPAN.
cheers,
pek
mackler
the pulldown menus show two-column full names from related tables.
Not only that, you made the change in less time than it took me to
compose the original question. Thank you!!
Adam
On Jan 19, 12:11 am, Peter Karman <pek...@gmail.com> wrote:
> I think I've just patched Rose::HTMLx::Form::Related to move toward
> that goal. Please try it from svn trunk here:
>
> https://svn.msi.umn.edu/sw/perl/
>
> You can see the mod I just made here:
>
> https://trac.msi.umn.edu/trac/sw/changeset/604
> let me know how it works for you and I'll push a 0.19 to CPAN.
>
> cheers,
> pek
Peter Karman
On Jan 19, 1:39 pm, mackler <adammack...@gmail.com> wrote:
> It works! The two-column full name is displayed on the edit page AND
> the pulldown menus show two-column full names from related tables.
> Not only that, you made the change in less time than it took me to
> compose the original question. Thank you!!
>
Cool. I've uploaded Rose::HTMLx::Form::Related 0.19 to CPAN.