in reply to Resistor network simplifier

Hello roboticus,

I know next-to-nothing about electronics, so I can’t comment on the substance of your code. But...

Comments about my coding style are always welcome

...well, since you ask ;-), I think your OO design could be improved.

(1) Statements like delete $g->{nodes}{out_neg}; violate encapsulation. So does using a return bless ..., "Gr"; statement in a non-Gr function such as build_impedence.

(2) The 4 bless ... "Gr" statements effectively create 4 separate constructors, with different names, which is at best confusing. But the real problem here is the superfluous creation of new objects. There is usually no need to return an object from a method call. Instead of:

$atten = build_pad(10); $atten->dump("PAD 10dB"); my $atten2 = build_pad(20); $atten2->dump("PAD 20dB"); $term = build_impedance(50); $term->dump("TERM 50ohm"); $g = $atten2->attach($term, [qw( out_pos in_pos )], [qw( out_neg in_ne +g )]); delete $g->{nodes}{out_neg}; delete $g->{nodes}{out_pos}; $g->dump("PAD 20dB + TERM 50ohm"); $h = $atten->attach($atten2, [qw( out_pos in_pos )], [qw( out_neg in_n +eg )]); delete $h->{nodes}{out_neg}; delete $h->{nodes}{out_pos}; $h->dump("PAD 10dB + PAD 20dB + TERM 50ohm"); my $i = $h->simplify(); $i->dump("RESULT!");

a cleaner interface would allow:

$atten = Gr->new(pad => 10); $atten->dump("PAD 10dB"); my $atten2 = Gr->new(pad => 20); $atten2->dump("PAD 20dB"); $term = Gr->new(impedance => 50); $term->dump("TERM 50ohm"); $atten2->attach($term, [qw( out_pos in_pos )], [qw( out_neg in_neg )]) +; $atten2->delete('nodes', 'out_neg'); $atten2->delete('nodes', 'out_pos'); $atten2->dump("PAD 20dB + TERM 50ohm"); $atten->attach($atten2, [qw( out_pos in_pos )], [qw( out_neg in_neg )] +); $atten->delete('nodes', 'out_neg'); $atten->delete('nodes', 'out_pos'); $atten->dump("PAD 10dB + PAD 20dB + TERM 50ohm"); $atten->simplify(); $atten->dump("RESULT!");

with all the gory details of object manipulation — including cloning, where required — confined to the Gr class.


BTW, this line of code:

my ($in_neg, $in_pos, $out_neg, $out_pos) = (node(), node(), node(), n +ode());

struck me as a challenge: is it possible to remove the duplicate calls? The obvious ... = (node()) x 4 won’t work, because the node() function has to be called four times, not just once. With a little trial-and-error, I found (to my surprise) that this does work (tested on Strawberry Perls 5.20.0 and 5.28.0):

$$_ = node() for \my ($in_neg, $in_pos, $out_neg, $out_pos);

Hope that’s of interest,

Athanasius <°(((><contra mundum Iustus alius egestas vitae, eros Piratica,

Replies are listed 'Best First'.
Re^2: Resistor network simplifier (updated)
by AnomalousMonk (Archbishop) on Oct 14, 2018 at 20:30 UTC
    $$_ = node() for \my ($in_neg, $in_pos, $out_neg, $out_pos);

    Or more simply, since  $_ is already aliased to each item of the list:

    c:\@Work\Perl\monks>perl -wMstrict -MData::Dump -le "use 5.010; ;; sub node { state $x = 42; return ++$x; } ;; $_ = node() for my ($in_neg, $in_pos, $out_neg, $out_pos); dd $in_neg, $in_pos, $out_neg, $out_pos; " (43 .. 46)

    Update 1: BTW: Both approaches work under Perl version 5.8.9 (without the use of state of course!).

    Update 2: Although I have to say that the first | quoted form could be very useful when iterating over a list of references to arbitrary my variables (actually, I think this would work with any kind of variable, but I haven't tested this):

    c:\@Work\Perl\monks>perl -wMstrict -le "use Data::Dump qw(dd); ;; print 'perl version: ', $]; ;; init($_) for \my ($scalar, @ra, %ha); dd $scalar, \@ra, \%ha; ;; sub init { my ($r) = @_; ;; return 'ARRAY' eq ref $r ? @$r = (9, 8, 7) : 'HASH' eq ref $r ? %$r = qw(one 1 two 2 three 3) : 'SCALAR' eq ref $r ? $$r = 42 : die qq{unknown ref type: '$r'} ; } " perl version: 5.008009 (42, [9, 8, 7], { one => 1, three => 3, two => 2 })


    Give a man a fish:  <%-{-{-{-<

Re^2: Resistor network simplifier
by roboticus (Chancellor) on Oct 14, 2018 at 19:38 UTC

    Athanasius:

    Thanks for taking the time to take a look at it.

    Regarding encapsulation violations: One of them was accidental. After I got the code largely to where I wanted it, I didn't like the delete $g->{nodes}{out_neg} statements, so I created _keep_only_names() to handle that. I put it into the first example, but forgot to put it into the second example. After thinking about it, I think that a better approach would be to tell the simplify() function which names it needs to care about, so I can use the same graph for multiple runs. If I ever get serious about putting more electronics components in there and other network types, I'll probably do something like that.

    For the second one, as you mention, I should put the factory functions build_impedance() and build_pad() into the Gr package.

    I don't generally write OO code in perl (aside from using packages from CPAN), as generally I use perl for my "let's learn about some problem domain before getting serious" language. I find good OO code really requires you to have a sufficient understanding of the problem domain before you start figuring out what the classes are or should be. I don't often get to use perl at the $job, so I generally use perl to explore the problem, then switch to an "approved" language, with classes and objects and whatnots ;^).

    Your solution to removing the duplicate node() calls is amusing looking. I may have to play with it a little and see how I like it.

    ...roboticus

    When your only tool is a hammer, all problems look like your thumb.

      If you're making graphs, I'd encourage you to use the Graph module (disclosure: I'm the maintainer). All the bookkeeping is done for you, you can add "attributes" to vertices or edges, and you can stringify them for instant debugging gratification.