FAQ
I'm hoping to use custom accessors to do some validation of incoming
data with reference to existing column data.

I have three fields: foo_enabled, bar_enabled, and default_view.
Constraint 1: one of foo_enabled or bar_enabled must be on/1 (i.e.
neither off/0)
Constraint 2: default_view can take either of 'foo' or 'bar' (not null etc)
Constraint 3: default_view can only take its value if the
corresponding {foo,bar}_enabled is set

There's a deadlock here if you can't defer checking. Say we have
foo_enabled=1, bar_enabled=0, default_view=foo then flip to
foo_enabled=0, bar_enabled=1, default_view=bar we'd have to order our
accessor calls to have both 1 (which is annoying).

One option seemed to me to have a pre-update() check rather than
having it in the accessor.

What do folks to here?

Paul, who skimmed ::Cookbook & ::ResultSource but didn't see anything jump out

PS Here's some code to see if I'm at least on the right track as far as it goes,

__PACKAGE__->add_columns(
bar_enabled => { accessor => '_bar_enabled' },
foo_enabled => { accessor => '_foo_enabled' },
default_view => { accessor => '_default_view' },
);

sub default_view {
my ($self, $value) = @_;

if (defined $value) {
unless ($value ~~ ['foo', 'bar']) {
die "default_view must be one of 'foo', 'bar'";
}
foreach my $view qw(foo bar) {
if ($value eq $view && !$self->"${view}_enabled") {
die "${view}_enabled must be set before default_view can be $view";
}
}
$self->_default_view($value);
}
return $self->_default_view;
}


sub bar_enabled {
my ($self, $value) = @_;

if (defined $value) {
if (!$value) { # switching if off
if (!$self->_foo_enabled) {
die "bar_enabled cannot be 0 if foo_enabled is 0";
}
unless ($self->_default_view ne 'bar') {
die "bar_enabled cannot be 0 if default_view is bar";
}
}
$self->_bar_enabled($value);
}
return $self->_bar_enabled;
}


sub foo_enabled {
my ($self, $value) = @_;

if (defined $value) {
if (!$value) { # switching if off
if (!$self->_bar_enabled) {
die "foo_enabled cannot be 0 if bar_enabled is 0";
}
if ($self->_default_view eq 'foo') {
die "foo_enabled cannot be 0 if default_view is foo";
}
}
$self->_foo_enabled($value);
}
return $self->_foo_enabled;
}

(If there's a cute way of avoiding that copy&paste I'm all ears, it's
been a long night :))

Search Discussions

  • Rob Kinyon at Jul 30, 2010 at 8:48 pm

    On Fri, Jul 30, 2010 at 16:31, Paul Makepeace wrote:
    I'm hoping to use custom accessors to do some validation of incoming
    data with reference to existing column data.
    [snip lots of explanation that isn't helping much]

    Why on earth are you setting stuff manually? It's obvious that you
    have business-level actions you want to take. So, code _THOSE_ up and
    have those do the right settings for foo_enabled, bar_enabled, and
    default_view. If someone wants to set things to "illegal" values, they
    should be able to. But, you'll be able to find those very easily with
    grep because they're the only ones actually using foo_enabled,
    bar_enabled, and/or default_view. All code outside the ResultSource
    should use set_view_to() or whatever.

    Remember - all problems are solvable by adding an additional layer of
    abstraction (except, of course, the problem of too many layers of
    abstraction).

    --
    Thanks,
    Rob Kinyon
  • Paul Makepeace at Jul 30, 2010 at 10:19 pm

    On Fri, Jul 30, 2010 at 13:48, Rob Kinyon wrote:
    On Fri, Jul 30, 2010 at 16:31, Paul Makepeace wrote:
    I'm hoping to use custom accessors to do some validation of incoming
    data with reference to existing column data.
    [snip lots of explanation that isn't helping much]

    Why on earth are you setting stuff manually? It's obvious...
    (Without the condescending tone it just wouldn't get through their
    thick skull, right?)
    So, code _THOSE_ up and
    have those do the right settings for foo_enabled, bar_enabled, and
    default_view. If someone wants to set things to "illegal" values, they
    should be able to. But, you'll be able to find those very easily with
    grep because they're the only ones actually using foo_enabled,
    bar_enabled, and/or default_view. All code outside the ResultSource
    should use set_view_to() or whatever.
    I'd be interested to hear what you think that abstracted method would be called.

    I'm not seeing an abstraction that simultaneously requires setting two
    booleans and a string to the name of one of them that doesn't end up
    being clunky or overly magical. On top of that, requires extra work in
    the controller presenting those vars from the form to the model.

    Compare,

    $r->foo_enabled(..params('foo_enabled'));
    $r->bar_enabled(..params('foo_enabled'));
    $r->default_view(..params('default_view'));

    with,

    $r->wtf(foo => ..params('foo_enabled'), bar =>
    ..params('foo_enabled'), default_view => ..params('default_view'));

    Not horrid, but definitely 'meh', and nothing's occurring to me at all
    what "wtf" should be called.

    Oh, and there's still having to override the accessors to die on any
    attempt to set a value thru' them.

    Catching the update is looking pretty good.

    Paul
    Remember - all problems are solvable by adding an additional layer of
    abstraction (except, of course, the problem of too many layers of
    abstraction).

    --
    Thanks,
    Rob Kinyon

    _______________________________________________
    List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
    IRC: irc.perl.org#dbix-class
    SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
    Searchable Archive: http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk
  • Rob Kinyon at Jul 31, 2010 at 1:42 am

    On Fri, Jul 30, 2010 at 18:19, Paul Makepeace wrote:
    On Fri, Jul 30, 2010 at 13:48, Rob Kinyon wrote:
    On Fri, Jul 30, 2010 at 16:31, Paul Makepeace wrote:
    I'm hoping to use custom accessors to do some validation of incoming
    data with reference to existing column data.
    [snip lots of explanation that isn't helping much]

    Why on earth are you setting stuff manually? It's obvious...
    (Without the condescending tone it just wouldn't get through their
    thick skull, right?)
    Believe it or not, the condescending tone doesn't come naturally to me.
    So, code _THOSE_ up and
    have those do the right settings for foo_enabled, bar_enabled, and
    default_view. If someone wants to set things to "illegal" values, they
    should be able to. But, you'll be able to find those very easily with
    grep because they're the only ones actually using foo_enabled,
    bar_enabled, and/or default_view. All code outside the ResultSource
    should use set_view_to() or whatever.
    I'd be interested to hear what you think that abstracted method would be called.
    How about enable_foo() and enable_bar()?
    I'm not seeing an abstraction that simultaneously requires setting two
    booleans and a string to the name of one of them that doesn't end up
    being clunky or overly magical. On top of that, requires extra work in
    the controller presenting those vars from the form to the model.

    Compare,

    ?$r->foo_enabled(..params('foo_enabled'));
    ?$r->bar_enabled(..params('foo_enabled'));
    ?$r->default_view(..params('default_view'));

    with,

    ?$r->wtf(foo => ..params('foo_enabled'), bar =>
    ..params('foo_enabled'), default_view => ..params('default_view'));
    You're not approaching this with an open mind. Of course, please note
    that I have ABSOLUTELY NO F'ING CLUE what your actual foo and bar are.
    Your original explanation was not quite crystalline in its clarity
    when describing the business cases.

    As for the code, I would envision it behaving something like this:

    You provide a table with a header of 3 columns and two rows of data
    looking something like:

    <table>
    <th>
    <td>View Name</td><td>Enabled</td><td>Default</td>
    </th>
    <tr>
    <td>Foo</td><td><checkbox></td><td><radiobutton></td>
    </tr>
    <!-- Repeat for bar -->
    </table>

    You would also have some JS that would do the following:
    1) Link the radio buttons as if they were in a group
    2) Mark a given button as disabled if its checkbox is turned off
    3) If a radiobutton is disabled when it is the set value, then the
    default goes "elsewhere"
    4) If a checkbox is unchecked and it was the only one checked,
    alert that this is unacceptable and disable the submit button

    Then, in your server code, you would do something like:

    foreach my $name ( qw( foo bar ) ) {
    if ( $.params("checked_$name") ) {
    $row->enable_foo( is_default => $.params("default_$name") );
    }
    }
    $row->update; # Automatically checks for dirtiness so you don't have to.
    Not horrid, but definitely 'meh', and nothing's occurring to me at all
    what "wtf" should be called.
    Now, you have an API on $row that maps how the user is thinking of the UI.
    Oh, and there's still having to override the accessors to die on any
    attempt to set a value thru' them.
    NO! You do _NOT_ want to do this. You want to have a "backdoor" that
    allows you to do something in a script that your application shouldn't
    be allowed to do. For example, if you want to convert a new client
    from their old application to yours. Their former provider may have
    been an idiot and allowed them to have no views enabled for a given
    row. Clearly, this is idiotic, but it is a state that needs to be
    loadable.

    Basically, putting those sort of overridden accessors means you don't
    trust your programmers. That's not Perl - that's Java. You're support
    to trust the coders and distrust the users.
    Catching the update is looking pretty good.
    There are very few reasons to override update. If you find yourself
    trying to do so, that's generally a good sign you're doing something
    wrong. It's only after you have exhausted every other possible path
    that overriding update should be considered as a good idea.

    --
    Thanks,
    Rob Kinyon
  • Ian at Jul 31, 2010 at 1:04 pm
    Paul.
    On 30/07/2010 21:31, Paul Makepeace wrote:
    I'm hoping to use custom accessors to do some validation of incoming
    data with reference to existing column data.

    I have three fields: foo_enabled, bar_enabled, and default_view.
    Constraint 1: one of foo_enabled or bar_enabled must be on/1 (i.e.
    neither off/0)
    Constraint 2: default_view can take either of 'foo' or 'bar' (not null etc)
    Constraint 3: default_view can only take its value if the
    corresponding {foo,bar}_enabled is set

    There's a deadlock here if you can't defer checking. Say we have
    foo_enabled=1, bar_enabled=0, default_view=foo then flip to
    foo_enabled=0, bar_enabled=1, default_view=bar we'd have to order our
    accessor calls to have both 1 (which is annoying).
    The bottom line is that this is business logic and so does not live in
    the controller. (I assume from your example that it is an MVC Controller)

    You are trying to apply the logic by restricting how the database
    accessors are updated. It is difficult to say from your description if
    it belongs in the database abstraction layer either (it could be argued
    either way).

    Much simpler and clearer to put an abstraction layer between the
    controller and model which is responsible for taking a number of values,
    checking the relative integrity of the values, then updating the
    database. This abstraction layer may simply be methods in the DBIx
    resultset or the result modules or can in more complicated examples be a
    complete new module.

    I would argue that having *any* dbix create/update/delete/find etc.
    methods in a controller is bad practice.

    e.g.

    # Call business logic which dies if anything is wrong.
    eval {
    my_business_logic_method({
    foo = ..params('foo_enabled'),
    bar = ..params('bar_enabled'),
    default_view = ..params('default_view'),
    });
    };
    if ($@) {
    print STDERR "my business logic has failed reason code $@\n";
    }

    I would argue that this is *much* clearer and although you may not agree
    since it is a little more verbose than your example, terseness is not
    always a virtue.

    Regards
    Ian

    <snip>
  • Piotr pogorzelski at Aug 2, 2010 at 8:36 am

    I would argue that having *any* dbix create/update/delete/find etc.
    methods in a controller is bad practice.
    I agree.

    in controller i also do translation of web parameters names to business
    parameters names.

    but what about user authorization check, which depends
    on business data and business logic.

    it should be done in controller?
    (otherwise session data should be send to model as well)

    --
    regards
    piotr

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupdbix-class @
categoriesperl, catalyst
postedJul 30, '10 at 8:31p
activeAug 2, '10 at 8:36a
posts6
users4
websitedbix-class.org
irc#dbix-class

People

Translate

site design / logo © 2022 Grokbase