Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced?

bartender1382 has asked for the wisdom of the Perl Monks concerning the following question:

Replies are listed 'Best First'.
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by hippo (Bishop) on May 05, 2022 at 17:22 UTC
Could I have written it better, cleaner, or safer? Better and cleaner, yes. Your $data is an array of arrays and you convert it to an array of hashes by hand. Far better to have DBI do that for you. `return $dbHandle->fetchall_arrayref ({});` [download] BTW, I would rename that variable. $db is your DB handle. What you have called $dbHandle is a statement handle. Anyone else coming to this code would be confused by that. 🦛	[reply] [d/l]
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by stevieb (Canon) on May 05, 2022 at 19:50 UTC
Excellent feedback so far. The only thing I'd add is to lose the comments. Let the code (ie. the proper naming of variables and function names) do the saying of what is happening. Comments should only be used to describe why something is happening. A comment in code should be a rather rare occurrence. If I see a comment in code, it's to point out why the following code is doing something that goes against the norm. For example: `### execute command ### get the data $dbHandle=$db->prepare($dbCommand); $dbHandle->execute(); my $data = $dbHandle->fetchall_arrayref();` [download] It's readily apparent that `execute()` and `fetchall_arrayref()` are performing the "execute command" and "get the data". I know it's instinctive when first writing code to pepper it with comments, but trust me, if you practice without them, you'll become more efficient as a code reader and writer more quickly. It will force you to come up with more descriptive names for various items within your code. Example: `my $var1 = 0; my $var2 = 1; # Draw an axis with a pencil and a ruler function($var1, $var2);` [download] vs: `my $pencil = 0; my $ruler = 0; draw_axis($pencil, $ruler); sub draw_axis { my ($pencil, $ruler) = @_; if (! $pencil) { # On rare occasions, a pencil might be broken, so do this $pencil++; } }` [download]	[reply] [d/l] [select]
Re^2: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by misterperl (Pilgrim) on May 05, 2022 at 19:54 UTC
I like your "style" sir - I do that but I realize I could do MORE of that. Making perl "english-like" and avoiding loops make life so much more readable.. AND using strict and bareblocks to minimize scope.. I ++'d you :)	[reply]
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by choroba (Cardinal) on May 05, 2022 at 15:37 UTC
Do you really need to fetch the whole array before iterating it? `$dbHandle->execute(); while (my @row = $dbHandle->fetchrow_array) { ... }` [download] `map{substr$_->[0],$_->[1]\|\|0,1}[\\|\|{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^ARGV,3]`	[reply] [d/l] [select]
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by misterperl (Pilgrim) on May 05, 2022 at 15:56 UTC
OK a few observations: `### grabbing every column my $dbCommand = qq^select ^; foreach (@ticketsFields) { $dbCommand .= "$_,"; } $dbCommand =~ s/,$//;` [download] ok I think all of that is simply: `my $dbCommand = 'select '.join ',',@ticketsFields;` [download] Why do you need this copy? and deref doesn't need {} , just use @$rows .. `my @newArray = @{$rows};` [download] All of this: `for(my $x = 0; $x < $ticketsFieldsSize; $x++) { ### assign the key and value in the hash $GenericData{$ticketsFields[$x]} = $newArray[$x]; }` [download] might be something like: `@GenericData{@ticketsFields} = @$rows;` [download] and lastly, you don't need a "return" at the end- whatever value is at the end, IS returned: `\@ticketsRecords; }` [download]	[reply] [d/l] [select]
Re^2: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by bartender1382 (Beadle) on May 05, 2022 at 19:57 UTC
Thank you for this: `@GenericData{@ticketsFields} = @$rows;` It works great, just would like to understand it. Why isn't it? %GenericData{@ticketsFields} = @$rows; Since I am building a hash, I would have just assumed the above. Just want to understand why it's being called as an array instead of a hash. Thanks again	[reply] [d/l]
Re^3: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by GrandFather (Saint) on May 05, 2022 at 21:58 UTC
`$, % and @` are sigils. They denote the type of thing being returned from an expression. They are not the type of a variable. So `$rows` is a variable ('rows') returning a scalar value. `@$rows` is a scalar variable deferenced to return an array. You can index into a hash variable (`%GenericData` for example) to get a scalar value - the value associated with the key that was used: `my $value = $GenericData{key};`. 'GenericData' in that case is the hash variable, but the indexing expression returns a scalar value. Perl also lets you slice hashes and arrays. In that case an array of values is returned so regardless of it being an array slice or a hash slice `@` is used to show an array being returned: `my @values = @GenericData{@ticketsFields}`. An interesting and useful wrinkle is that you can then assign an array to the slice. Consider the following example which reverses the order of values associated with keys in a hash using assignment between hash slices: `use strict; use warnings; my %hash = (one => 1, two => 2, three => 3); my @keys = sort {$hash{$a} <=> $hash{$b}} keys %hash; print "$_ => $hash{$_} " for @keys; print "\n"; @hash{@keys} = @hash{reverse @keys}; print "$_ => $hash{$_} " for @keys;` [download] Prints: `one => 1 two => 2 three => 3 one => 3 two => 2 three => 1` [download] Optimising for fewest key strokes only makes sense transmitting to Pluto or beyond	[reply] [d/l] [select]
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by Fletch (Bishop) on May 05, 2022 at 18:41 UTC
Couple of tangential notes: Be very careful where whatever input you're using is coming from that you're building your SQL `select` statement up otherwise you're asking for an injection attack. Also if you used join to build your list of column names (`join(q{,},@ticketsFields)`) you don't have to go back and clean up the extraneous trailing comma (and that's still not going to insulate from injection problems if someone can get "garbage" into the list). The cake is a lie. The cake is a lie. The cake is a lie.	[reply] [d/l] [select]
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by tangent (Parson) on May 06, 2022 at 00:10 UTC
I may be missing something but you can get DBI to do all this work for you. Using selectall_arrayref() with the 'Slice' option will return an array reference of hash references the same as your `return \@ticketsRecords`: `### execute command ### get the data my $data = $db->selectall_arrayref( $dbCommand, { Slice => {} } ); return $data;` [download]	[reply] [d/l] [select]
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by tybalt89 (Monsignor) on May 05, 2022 at 15:54 UTC
Untested, of course... #!/usr/bin/perl use strict; # https://perlmonks.org/?node_id=11143595 use warnings; ###### from my Perl pm module our @ticketsFields = qw( ... every column in my table...); ### number of columns #our $ticketsFieldsSize = @ticketsFields; our @ticketsRecords; sub getTicketsRecords { ### my conditional arguments ### to be used in WHERE and AND my (...multiple args... ) = @_; ### build select command ### grabbing every column my $dbCommand = qq^select ^ . join ',', @ticketsFields; # foreach (@ticketsFields) { # $dbCommand .= "$_,"; # } # $dbCommand =~ s/,$//; ### and the conditional arguments passed in $dbCommand .= ... ### execute command ### get the data $dbHandle=$db->prepare($dbCommand); $dbHandle->execute(); my $data = $dbHandle->fetchall_arrayref(); $dbHandle->finish; ### go throught every row, in the reference array foreach my $rows ( @$data ) { ### dereference the first/next row # my @newArray = @{$rows}; ### create a blank hash my %GenericData; @GenericData{ @ticketsFields } = @$rows; # NOTE added ### loop through the new row ### for every column in the table # for(my $x = 0; $x < $ticketsFieldsSize; $x++) # { # ### assign the key and value in the hash # $GenericData{$ticketsFields[$x]} = $newArray[$x]; # } ### add reference to has to an array push @ticketsRecords, \%GenericData; } ### pass back a reference to my array of hash references return \@ticketsRecords; } [download]	[reply] [d/l]
Re^2: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by stevieb (Canon) on May 05, 2022 at 22:27 UTC
It would be prudent and helpful to explain what you're replacing, and why.	[reply]
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by BillKSmith (Monsignor) on May 06, 2022 at 14:10 UTC
The use of the function map rather than `foreach...` and `zip` (from List::MoreUtils) eliminates the need for all your temporary variables and the associated reference/dereference. `use List::MoreUtils qw(zip); my $TicketRecords = [] ; @{$TicketRecords} = map { { zip @TicketFields, @$_ } } @$data;` [download] Bill	[reply] [d/l] [select]
Re: Is there a cleaner way to write this code below, which includes derferencing references, that themselves have to be dereferenced? by Anonymous Monk on May 06, 2022 at 14:08 UTC
Because this is Perl, There Is More Than One Way To Do It (whatever "It" happens to be.) Another way to build the query string is: my $dbCommand = do { local $" = ','; "select @ticketsFields ..."; }; The `$"` variable determines what gets put between elements when you interpolate an array. If you `use English;` you can spell this `$LIST_SEPARATOR`. No matter which spelling you use you should localize the value to avoid spooky action at a distance, since this variable is global. The `do` block limits the scope of the local change to `$"`. I do not present this as a better way to build the query string, simply as another way. Nitpick (or maybe not): Unless variables like `@ticketsFields` actually need to be visible outside your module, you could (and probably should) specify them as `my` rather than `our`. This may actually have practical consequences in the case of `@ticketsRecords`. Since you declared it `our`, there is only one instance of it. So if you do something like my $rec_1 = getTicketsRecords( ... ); my $rec_2 = getTicketsRecords( ... ); `$rec_1` and `$rec_2` refer to the same array, and contain the same data -- the results of the most recent call to `getTicketRecords()`. If you say `my @ticketsRecords`, each call returns a reference to a different array.	[reply] [d/l] [select]


Clear questions and runnable code get the best and fastest answer
	PerlMonks