Iterate over keys or values of hash, not hash itself

[stefanhaerter]

At a few places, for loops are iterating over a hash, not keys or values. While this is functionaly fine because everywhere there is a check if the iterator is a valid key of the hash, I would consider it a bad coding practice and thus fix it nonetheless.

[stefanhaerter]

On rel-11_0, I already identified the relevant places (see below), but I also want to check rel-10_1 and rel-10_0, therefor PR has to wait a bit.

• Kernel/Output/HTML/TicketOverview/Preview.pm
  

  otobo/Kernel/Output/HTML/TicketOverview/Preview.pm

  Lines 276 to 283 in c0a2b2f

  	my $ArticleObject = $Kernel::OM->Get('Kernel::System::Ticket::Article');
  	my $PreviewArticleSenderTypes = $ConfigObject->Get('Ticket::Frontend::Overview::PreviewArticleSenderTypes');
  	my %PreviewArticleSenderTypeIDs;
  	if ( IsHashRefWithData($PreviewArticleSenderTypes) ) {
  	KEY:
  	for my $Key ( %{$PreviewArticleSenderTypes} ) {
  	next KEY if !$PreviewArticleSenderTypes->{$Key};

• Kernel/System/Auth/OpenIDConnect.pm
  

  otobo/Kernel/System/Auth/OpenIDConnect.pm

  Lines 447 to 449 in c0a2b2f

  	KEY:
  	for my $Key ( %{ $Param{Map} } ) {
  	if ( IsHashRefWithData( $Param{Map}{$Key} ) ) {

• Kernel/System/Package.pm
  

  otobo/Kernel/System/Package.pm

  Lines 2380 to 2389 in c0a2b2f

  	TAG:
  	for my $Item (qw(DatabaseInstall DatabaseUpgrade DatabaseReinstall DatabaseUninstall)) {
  	if ( ref $Param{$Item} ne 'HASH' ) {
  	next TAG;
  	}
  	for my $Type ( sort %{ $Param{$Item} } ) {
  	if ( $Param{$Item}->{$Type} ) {

• Kernel/System/ZnunyHelper.pm
  

  otobo/Kernel/System/ZnunyHelper.pm

  Lines 678 to 690 in c0a2b2f

  	my %ScreenConfig = %Param;
  	my @Settings;
  	my $NoConfigRebuild = 0;
  	if ( $Param{NoConfigRebuild} ) {
  	$NoConfigRebuild = 1;
  	delete $Param{NoConfigRebuild};
  	}
  	VIEW:
  	for my $View (%ScreenConfig) {
  	next VIEW if !IsHashRefWithData( $ScreenConfig{$View} );

  
  

  otobo/Kernel/System/ZnunyHelper.pm

  Lines 826 to 831 in c0a2b2f

  	my %ScreenConfig = %Param;
  	VIEW:
  	for my $View (%ScreenConfig) {
  	next VIEW if !IsHashRefWithData( $ScreenConfig{$View} );

[bschmalhofer]

Yes, these loops are strange. @stefanhaerter, could you check whether there is a Perl::Critic policy that covers these cases?

[stefanhaerter]

Yes, I think this is a very good idea - I thought about writing a CodePolicy plugin by myself, but if Perl::Critic already covers this, this would be great.

[stefanhaerter]

@bschmalhofer At a quick glanze, I couldn't find anything that matches this need at https://metacpan.org/dist/Perl-Critic/view/lib/Perl/Critic/PolicySummary.pod. Perhaps a plugin is needed.

[stefanhaerter]

Closed with merging the PRs.