I don't want to sound harsh, but your code makes my eyes bleed. The good news is that there are a few easy steps you can take to make it much easier to read, understand, and maintain.
Use descriptive variable names. Names like %Myhash are fine in CS textbook examples, but in real code they are pure evil. What kind of information does the hash contain? Put it in the name. Something like %system_info can be much easier to understand.
Use consistent capitalization and formatting in your variable names. Find a style you like and stick to it. Generally variable names are lower_case_with_underscores but some people prefer initialLowerCaseWithCamelCaseAfter. Take a look at perlstyle.
Banishing $iteminiteminiteminitem and friends will take care of 90% of the problem with this code. What else can we do to fix it?
Format your code for readability. Break up long lines. Use whitespace. Use consistent indentation.
foreach $item (keys %Myhash){
if ($item eq $My_Value) {
foreach $iteminitem (sort(keys %{$Myhash{$item}})){
foreach $iteminitem1 (sort(keys %{$Myhash{$item}{$iteminit
+em}})){
if(
$iteminitem1 eq 'name'
&&
$user_number == $Myhash{$item}{$iteminitem}{$itemi
+nitem1}
) {
my $User = $iteminitem;
}
}
}
}
}
All I did was reformat your code and it is a fair bit easier to read. You might want to take a look at perltidy. It does a pretty good job of reformatting perl code for you. Some people require that all code be passed through perltidy before check in to their source repositories.
Remember that you should be writing your code so that you will understand it 6 months from now, when you get a call at 4am on a Saturday morning about the emergency bug that must be fixed right away.
Well written code is primarily a document written to communicate the functioning of the system for maintenance programmers. The fact that it runs and does work is just a side effect of a well documented system.
OK, the last statement isn't really true, but I've found that keeping this point of view in mind helps to improve the readability of my code.
After cleaning up your code a bit, I noticed that you seem to be doing some unnecessary loops.
# This:
foreach my $top_item ( keys %Myhash ){
if ($top_item eq $My_Value) {
# do stuff
}
}
# can be reduced to this:
if ( exists $Myhash{$My_Value} ) {
# do stuff
}
# And this:
foreach $iteminitem1 (sort(keys %{$Myhash{$item}{$iteminitem}})){
if(
$iteminitem1 eq 'name'
&&
$user_number == $Myhash{$item}{$iteminitem}{$itemi
+nitem1}
) {
my $User = $iteminitem;
}
}
# can be reduced to this:
if (
exists $Myhash{$My_Value}{$iteminitem}{'name'}
and
$user_number == $Myhash{$My_Value}{$iteminitem}{'name'}
) {
my $User = $iteminitem;
}
# The combined restructuring yields:
if ( exists $Myhash{$My_Value} ) {
foreach $iteminitem (sort(keys %{$Myhash{$item}})){
if (
exists $Myhash{$My_Value}{$iteminitem}{'name'}
and
$user_number == $Myhash{$My_Value}{$iteminitem}{'name'}
) {
my $User = $iteminitem;
}
}
}
That eliminates several unneeded sorts and a fair bit of looping over arrays.
Keep working on your code, you'll get better. Improving the readability of your code will make it easier to debug as you do your initial development and easier to add features and debug in the future. Learning a few simple habits now will increase your productivity, make your teeth whiter and brighter, and get you the girls.
|