Monks,
I have two complementary methods in a class. They pack a hash of values into a binary scalar and unpack it. One uses a format declaration to control the pack operation, the other uses a format declaration to control the unpack. The two subs "want" the same format declaration but so far I am having to declare it twice which has a nasty smell to it. In fact during my development so far I have already been bitten by inconsistent changes in the two declarations.
Update: further reading of the docs tells me that pack/unpack is not the way to do the bit-level twiddling. So I am looking into vec for the flag handling. But I suspect pack is still required for the other parts of the message structure, and the general query about redundant code still applies.
I am stuck trying to think of a way to make the redundant duplicate declarations into a single one that is accessible to both subs.
The code is below - I put in the whole of the two subs in order to avoid over-simplifying, as they are not 'too large'. Normal strictures and warnings apply. (I did have to edit after the copy/paste to remove trailing whitespace. Other than that, WhatYouSeeIsWhatIGot.)
#########
sub new {
#########
my $class = shift;
my $self = {};
bless ($self,$class);
my %fragArgs = %{ $_[0] }; # dereference the hashref of m
+essage fragment parameters
die "no message ID!" unless defined $fragArgs{msgID} ;
die "no frag seq!" unless defined $fragArgs{fragSeq} ;
die "no morefrag flag!" unless defined $fragArgs{moreFrags} ;
die "no frag data !" unless defined fragArgs{fragData} ;
&setUpLogging;
# Declare the constant field lengths for the pack statement
# to package the msg data into a fragment
my $fmtMsgID = 'H4';# 16 bits Msg ID 4 hex digits
my $fmtFragSeq = 'b7';# 7 bits Frag Seq
my $fmtMoreFrags = 'b'; # 1 bit More Frags flag
+
my $fmtFlagUnitIDPresent = 'b'; # 1 bit Unit ID Present
my $fmtFlagCRCPresent = 'b'; # 1 bit CRC Present
my $fmtFlagsUnused = 'b6'; # 6 bits Unused
+
my $fmtfragLen = 'H2'; # 8 bits Frag Length
+
#my $fmtFragData is 'variable';
my $fmtUnitID = 'A8'; # if present, unit ID is 8 chars
my $fmtFragCRC = 'H4'; # if present, CRC is a 16 bit word
+
# create the fragment packing format according to the data length an
+d flags
my $fragFormat =
$fmtMsgID .
$fmtFragSeq .
$fmtMoreFrags .
$fmtFlagUnitIDPresent .
$fmtFlagCRCPresent .
$fmtFlagsUnused .
$fmtfragLen .
'A' . (length $fragArgs{fragData} ) .
($fragArgs{unitID} ? $fmtUnitID : '') .
($fragArgs{doCRC} ? $fmtFragCRC: '') ;
#pack the individual bits of the first message hash according to the
+ pack-format.
# include Unit ID and/or CRC data if they are declared for this mess
+age.
DEBUG "packing message frag using format $fragFormat";
my $wrappedFrag = pack $fragFormat,
$fragArgs{msgID},
$fragArgs{fragSeq},
$fragArgs{moreFrags},
($fragArgs{unitID}? 1 : 0 ), # set UnitIDPresent bit if
+given
($fragArgs{CRC} ? 1 : 0 ) , # set CRCPresent bit if giv
+en
0, # 0 for the unused bits
+
length $fragArgs{fragData},
$fragArgs{fragData},
($fragArgs{unitID} ? $fragArgs{unitID} :''),
+# include unit ID if it was given
($fragArgs{doCRC} ? &doCRC($fragArgs{fragData})
+ :'');
$Data::Dumper::Sortkeys = $Data::Dumper::Terse = $Data::Dumper::Inde
+nt = 1;
print Dumper(\%fragArgs) . "gives:\n". Dumper($wrappedFrag);
return $wrappedFrag;
}
#############
sub unwrap {
#############
#given a msg fragment in packed binary return a message hash
my $class = shift;
my $self = {};
bless ($self,$class);
my $wrappedFrag = $_[0] ;
my %msgFrag = ();
die "no message frag!" unless defined $wrappedFrag;
&setUpLogging;
# Declare the constant field lengths for the unpack statement
# to extract the msg data from a fragment
my $fmtMsgID = 'H4';# 16 bits Msg ID
+
my $fmtFragSeq = 'b7';# 7 bits Frag Seq
+
my $fmtMoreFrags = 'b'; # 1 bit More Frags flag
+
my $fmtFlagUnitIDPresent = 'b'; # 1 bit Unit ID Present
+
my $fmtFlagCRCPresent = 'b'; # 1 bit CRC Present
+
my $fmtFlagsUnused = 'b6'; # 6 bits Unused
my $fmtfragLen = 'H2'; # 8 bits Frag Length
+
#my $fmtFragData is 'variable';
my $fmtUnitID = 'A8'; # if present, unit ID is 8 chars
my $fmtFragCRC = 'H4'; # if present, CRC is a 16 bit word
+
# We will have to take the frag apart in a number of bites.
# 1. Msg ID, Frag Seq, More Frags flag, Unit ID Present, CRC Present
+, Frag Length.
my $fragFormat =
$fmtMsgID .
$fmtFragSeq .
$fmtMoreFrags .
$fmtFlagUnitIDPresent .
$fmtFlagCRCPresent .
$fmtFlagsUnused .
$fmtfragLen ;
DEBUG "unpacking message frag using format $fragFormat";
($msgFrag{msgID},
$msgFrag{fragSeq},
$msgFrag{moreFrags},
$msgFrag{unitID},
$msgFrag{CRC},
my $junk, #for the unused flagbits
my $fragLength)
= unpack $fragFormat, $wrappedFrag;
$Data::Dumper::Sortkeys = $Data::Dumper::Terse = $Data::Dumper::Ind
+ent = 1;
print "unpack got \n" . Dumper(\%msgFrag) . "from:\n". Dumper($wrapp
+edFrag);
# further code to be added ...
return %msgFrag;
}
The pack format definition is deliberately built up from individual pieces because (i) formats vary from one message to another; (ii) the overall message-format design itself may be subject to change.
Clearly common side-effect-free actions can be subroutined out, as with &setUpLogging; (aside: I feel sheepish using the "&" form in present company, but I still find it helpful - if anyone proides a link to a 'use of "&" harmful' node I promise to go read it.)
But I do not see how I can do the same thing with all those
my $fmtFoo's.
The more I strive to do things "the right way", the more there is to learn. Please help to put me back on The Path.
This signature will be better than the next one