[Koha-bugs] [Bug 20551] Add option for including deleted records in export_records.pl

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20551

--- Comment #66 from David Gustafsson <glasklas at gmail.com> ---
(In reply to Marcel de Rooy from comment #57)
> +    my $marc_flavour = C4::Context->preference('marcflavour');
> +    my $biblio_metadata = $resultset->find({
> +        'biblionumber' => $biblionumber,
> +        'format' => 'marcxml',
> +        'marcflavour' => $marc_flavour
> +    });
> +    my $marc_xml = $biblio_metadata->metadata;
> +    $marc_xml = StripNonXmlChars($marc_xml);
> +
> +    my $record = eval {
> +        MARC::Record::new_from_xml($marc_xml, 'UTF-8', $marc_flavour)
> +    };
> +    if (!$record) {
> 
> You test $record, but you should have tested $biblio_metadata too ?
> Otherwise you already crash on ->metadata..

Hi, thanks for the feedback. There is no definite right or wrong, but
personally try to avoid defensive programming where things are very unlikely to
fail. Ids should be valid since have been previously fetched from database, in
case of the XML-decoding it's highly unlikely to go wrong, but I can imagine
cases where it could.(In reply to Marcel de Rooy from comment #58)
> +    return unless (@{$record_ids} || @{$deleted_record_ids} && $format ne
> 'csv');
> 
> Parentheses here would be more helpful inside. The outer ones are not needed.
> But in order to read easier, you could do A || ( B && C )

Yes,they could be removed. Personally I think it's more confusing, or at least
a bit unsightly, to use parenthesis for anything else than changing operator
precedence. At least with "||" and "&&" where there really should be no
confusion.

-- 
You are receiving this mail because:
You are watching all bug changes.