Beefy Boxes and Bandwidth Generously Provided by pair Networks
Pathologically Eclectic Rubbish Lister
 
PerlMonks  

Re: help with user selected hash operations?

by kcott (Archbishop)
on Oct 30, 2017 at 11:48 UTC ( [id://1202325]=note: print w/replies, xml ) Need Help??


in reply to help with user selected hash operations?

G'day lunette,

Welcome to the Monastery.

"i am at the end of my rope here, honestly ..."

Don't Panic! There are no insurmountable problems here.

"it feels like nothing is working. ..."

Most of the individual parts of your code are fine; in fact, for a raw novice, it's pretty good (I've certainly seen a lot worse). I get the impression that you've got the basic logic correct in your head — you just haven't translated that to Perl code.

In the following, I've aimed to highlight the good and the bad, and how to fix the latter. Your assignment may have read something like "Do A, B and C only using X, Y and Z", but you haven't told us that. You also haven't said what the main focus of the assignment was: are you learning about hashes? I/O? UI presentation? something else? If you have another question at a later date, that's useful information to include. I've tried to keep my suggestions in line with with the level of code you've presented; for instance, I haven't used any references or regular expressions. If I've used something you weren't supposed to use, and you can't determine an alternative, just ask.

The first thing I had a problem with was your indentation. Your first if block is like this:

if ($choice eq 'a'){ ... }

At first glance, it appears that '}' is the closing brace for the if block. But it's not! That's the closing brace for a different if block: one that appears in "...", indented two more levels, inside another line of code.

Indentation doesn't matter to Perl (it does in some other languages and file formats); it does, however, matter a lot to humans who have to read, and possibly modify, the code. You've done the same thing with "if ($choice eq 'd') ...". In both cases, Perl sees a code structure like this:

if (CONDITION1) { ... } if (CONDITION2) { ... }

However, what you really wanted was a code structure more like this:

if (CONDITION1) { ... if (CONDITION2) { ... } }

Fixing that code structure throughout, will resolve a lot of your current issues.

Also, instead of a series of if statements, all of whose conditions Perl has to check; consider the following construct, where conditions are only checked until one is found to be TRUE.

if (CONDITION1) { ... } elsif (CONDITION2) { ... } elsif (CONDITION3) { ... } else { ... }

There may be better ways to handle that but, as I said at the start, I'm aiming to keep things simple. See "perlintro: Conditional and looping constructs" for more about that.

[By the way, the expression if loop is wrong. If you call it a loop, and start thinking in terms of loops, you likely add another level of confusion.]

Another problem you have is with changing cases. It's good that you thought to canonicalise the input; unfortunately, you haven't got it quite right. If the user enters "Jeff", "JEFF", or any other combination (perhaps even something daft like "jEfF"), lc will change that to "jeff":

$ perl -E 'my @x = qw{Jeff JEFF jEfF}; say lc $_ for @x' jeff jeff jeff

But, "jeff" isn't one of the keys of '%son_father' — "Jeff" is though (oops!). You can fix that by also using ucfirst:

$ perl -E 'my @x = qw{Jeff JEFF jEfF}; say ucfirst lc $_ for @x' Jeff Jeff Jeff

Although there would be better ways to do that in general code, it works with the data you've presented, so I've avoided anything more complex. You can use it for both the keys and values. You do need both the ucfirst and the lc:

$ perl -E 'my @x = qw{Jeff JEFF jEfF}; say ucfirst $_ for @x' Jeff JEFF JEfF

If you deal with those two main areas, you can probably get your program up and running reasonably well. There are a few other areas which, while not actually breaking your current code, are poor choices, bad practices, and habits you don't really want to get into.

  • While exit will exit the loop, it also exits the entire program: no code after while would ever be executed. To exit just the loop, use last: that documentation has links to other, useful, loop control functions.
  • It's good that you've used lexical (my) variables; however, declaring them all at the start of your code means that all subsequent code has access to them and can change them. You should use a limited scope for all such variables. That may be a bit beyond where you're up to in your studies: see my example code below for how I've handled this; ask for more information if you're interested.
  • Choose more meaningful names for your variables. Reusing '$choice' throughout was a bad move anyway (as per my last point); however, to someone reading your code (including yourself in a few weeks or months time), it's very unclear what data that variable holds (menu selection? father's name? etc.) which makes it error-prone. I also found '%son_father' ambiguous: I had to look for usage in subsequent code before I was confident (i.e. no longer guessing) of its meaning.
  • Similar to the last point, present the user with meaningful information (in prompts, messages and so on). The prompt "Enter a male name: " is unclear to the user: given the chosen function, that could be the father or the son. Again, I had to look at surrounding code to understand the intent: users of your software don't have this option.
  • In general, avoid monolithic scripts. Abstract code into subroutines: you can then reuse them or, even if only used once, that code won't clutter the main logic. Again, this may be beyond where your studies are: ask for more information, if interested.

Here's an example script that just implements the "a" and "e" menu selections. It has examples of all the points I've made above.

#!/usr/bin/env perl use strict; use warnings; { my %father_of = get_son_to_father_map(); display_menu(); while (1) { my $menu_selection = get_menu_selection(); if ($menu_selection eq 'e') { print "Exiting ...\n"; last; } elsif ($menu_selection eq 'a') { print "Son-father pair addition.\n"; print "Enter son's name: "; chomp( my $son = ucfirst lc <STDIN> ); if (exists $father_of{$son}) { print "$father_of{$son} is the father of $son\n"; } else { print "Enter father's name: "; chomp( my $father = ucfirst lc <STDIN> ); $father_of{$son} = $father; } } else { print "Invalid menu selection! Try agin.\n"; } } print "Done!\n"; } sub display_menu { print <<'MENU'; Menu Selections =============== a - add son-father pair e - exit MENU return; } sub get_menu_selection { print "\nEnter menu selection: "; chomp( my $selection = lc <STDIN> ); return $selection; } sub get_son_to_father_map { return ( Jeff => 'Doug', Thomas => 'Evan', Robert => 'Jason', Bruce => 'Richard', Clark => 'Jon', ); }

Here's a sample run:

Menu Selections =============== a - add son-father pair e - exit Enter menu selection: qwerty Invalid menu selection! Try agin. Enter menu selection: Invalid menu selection! Try agin. Enter menu selection: a Son-father pair addition. Enter son's name: JEFF Doug is the father of Jeff Enter menu selection: a Son-father pair addition. Enter son's name: bRuCe Richard is the father of Bruce Enter menu selection: a Son-father pair addition. Enter son's name: fred Enter father's name: BILL Enter menu selection: a Son-father pair addition. Enter son's name: FrEd Bill is the father of Fred Enter menu selection: e Exiting ... Done!

— Ken

Replies are listed 'Best First'.
Re^2: help with user selected hash operations?
by lunette (Acolyte) on Oct 30, 2017 at 17:40 UTC

    oh, my gosh, this is more than i expected. thank you so much. T_T

    i was hesitant to post the entire code because i thought it might be too long, but with little edits i've made so far, this is what i have:

    #! /usr/bin/perl use strict; my %son_father; my $choice; my $name1; my $name2; my $name3; my $name4; my $newname; my $add_dad; %son_father = (Jeff => "Doug", Thomas => "Evan", Robert => "Jason", Bruce => "Richard", Clark => "Jon") ; my $menu = <<MENU; SON_FATHER Hash Operations a- Add a son-father pair d- Delete a son-father pair e- exit the program g- Get a father o- Output the hash neatly r- Replace a father x- get a grand father MENU while (1) { # display the menu print $menu, "\n\n"; # get the user choice print "Make your choice: "; chomp($choice = lc <STDIN>); # fulfill the user request if ($choice eq 'a'){ print "Enter a male name: "; chomp (my $name1 = ucfirst lc <STDIN>); } if (exists $son_father{$name1}) { print "Duplicate name -- try again!\n"; } else { print "Add a father: "; chomp (my $add_dad = ucfirst lc <STDIN>); $son_father{$name1} = {$add_dad}; next; } if ($choice eq 'd') { print "Enter a male name: "; chomp (my $name2 = ucfirst lc <STDIN>); } if (exists $son_father{$name2}) { delete $son_father{$name2}; } else { print "Sorry, couldn't find you -- try again later!"; next; } if ($choice eq 'e') { print "Come back again -- goodbye!"; last; next; } if ($choice eq 'g'){ print "Enter a son's name: "; chomp (my $name4 = ucfirst lc <STDIN>); } if (exists $son_father{$name4}) { print "$son_father{$name4}\n"; next; } if ($choice eq 'o'){ my ($key, $value); foreach $value (sort keys %son_father){ print "$value\t$son_father{$value}\n"; next; } } if ($choice eq 'r'){ print "Enter a male name: "; chomp (my $name3 = ucfirst lc <STDIN>); if (exists $son_father{$name3}) { print "Enter a new father name: "; chomp ($newname = ucfirst lc <STDIN>); $son_father{$name3} = {$newname}; } else { print "Sorry, couldn't find you -- try again later!"; next; } } if ($choice ne 'a', $choice ne 'r', $choice ne 'o', $choice ne 'd' +, $choice ne 'e', $choice ne 'g') { print "Invalid choice, TRY AGAIN!\n"; next; } }

    i can change my scalar variable names to make it easier to understand, these are just the names the professor asked us to use.

    the problem i seem to be having is that the secondary STDINs seem to trigger the "invalid response", even with "next" as part of the code. along with that, if i do the "a" command first, the "duplicate name" prompt shows up in subsequent command loops... on top of that, i keep getting HASH(x) in place of the names i'm putting in for the secondary STDINs in the "a" and "r" commands. like this:

    SON_FATHER Hash Operations a- Add a son-father pair d- Delete a son-father pair e- exit the program g- Get a father o- Output the hash neatly r- Replace a father x- get a grand father Make your choice: a Enter a male name: Bruce Duplicate name -- try again! Invalid choice, TRY AGAIN! SON_FATHER Hash Operations a- Add a son-father pair d- Delete a son-father pair e- exit the program g- Get a father o- Output the hash neatly r- Replace a father x- get a grand father Make your choice: a Enter a male name: Phil Add a father: Justin SON_FATHER Hash Operations a- Add a son-father pair d- Delete a son-father pair e- exit the program g- Get a father o- Output the hash neatly r- Replace a father x- get a grand father Make your choice: o Duplicate name -- try again! Bruce Richard Clark Jon Jeff Doug Phil HASH(0x6453d0) Robert Jason Thomas Evan Invalid choice, TRY AGAIN! SON_FATHER Hash Operations a- Add a son-father pair d- Delete a son-father pair e- exit the program g- Get a father o- Output the hash neatly r- Replace a father x- get a grand father Make your choice: e Duplicate name -- try again! Come back again -- goodbye!

    sorry, this is a lot. >_< i really do want to understand how to make this run perfectly, not just for my class, but just so i can use it again without getting this mixed up.

    and thank you again for being so helpful. it makes me feel really welcome, despite how clueless i can be, haha.

    EDIT: i forgot to mention - if i don't explicitly use "my (variable)" at the beginning of the code, i'm continuously told by my terminal AND my writing program that it isn't explicitly defined at all, regardless of if i used "my" within the chomp or not. i know i should be able to just use the "my" command within the STDIN perimeters, but no matter how i fiddle with it, it just tells me it's wrong!

      lunette:

      You didn't quite get what kcott was telling you about your if statements and indentation. I've reformatted the code a little to hopefully help you see what he means:

      $ cat pm_1202351.pl #! /usr/bin/perl use strict; my %son_father; my $choice; my $name1; my $name2; my $name3; my $name4; my $newname; my $add_dad; %son_father = (Jeff => "Doug", Thomas => "Evan", Robert => "Jason", Bruce => "Richard", Clark => "Jon") ; my $menu = <<MENU; SON_FATHER Hash Operations a- Add a son-father pair d- Delete a son-father pair e- exit the program g- Get a father o- Output the hash neatly r- Replace a father x- get a grand father MENU while (1) { # display the menu print $menu, "\n\n"; # get the user choice print "Make your choice: "; chomp($choice = lc <STDIN>); # fulfill the user request if ($choice eq 'a'){ print "Enter a male name: "; chomp (my $name1 = ucfirst lc <STDIN>); } if (exists $son_father{$name1}) { #46 print "Duplicate name -- try again!\n"; } else { print "Add a father: "; chomp (my $add_dad = ucfirst lc <STDIN>); $son_father{$name1} = {$add_dad}; next; } if ($choice eq 'd') { print "Enter a male name: "; chomp (my $name2 = ucfirst lc <STDIN>); } if (exists $son_father{$name2}) { #59 delete $son_father{$name2}; } else { print "Sorry, couldn't find you -- try again later!"; next; } if ($choice eq 'e') { print "Come back again -- goodbye!"; last; next; } if ($choice eq 'g'){ print "Enter a son's name: "; chomp (my $name4 = ucfirst lc <STDIN>); } if (exists $son_father{$name4}) { #76 print "$son_father{$name4}\n"; next; } if ($choice eq 'o'){ my ($key, $value); foreach $value (sort keys %son_father){ print "$value\t$son_father{$value}\n"; next; } } if ($choice eq 'r'){ print "Enter a male name: "; chomp (my $name3 = ucfirst lc <STDIN>); if (exists $son_father{$name3}) { print "Enter a new father name: "; chomp ($newname = ucfirst lc <STDIN>); $son_father{$name3} = {$newname}; } else { print "Sorry, couldn't find you -- try again later!"; next; } } if ($choice ne 'a', $choice ne 'r', $choice ne 'o', $choice ne 'd', +$choice ne 'e', $choice ne 'g') { print "Invalid choice, TRY AGAIN!\n"; next; } }

      Pay close attention to the lines marked 46, 59 and 76: You're intending them to be inside the prior if block, but it's hard to see that because your braces and indentation aren't consistent with each other.

      ...roboticus

      When your only tool is a hammer, all problems look like your thumb.

        oh! i'm so sorry, the confusing indentations are a habit my professor has, and i just picked it up as i was going. it is a bit easier to read like this, haha. thank you!

      "oh, my gosh, this is more than i expected. thank you so much. T_T"

      You're very welcome.

      Some further responses came to mind as I was reading your reply; however, it appears they've already been covered, so there's no need for me to repeat them. As the update to your initial question indicates that your code is now "working perfectly", I'll assume that additional advice has clarified any outstanding issues.

      You've apologised a number of times for the length of what you've posted. There's really no need: I don't see anything that's excessively long. In general, avoid adding anything not directly related to the question, and keep the code that does demonstrate the problem to a minimum. You can wrap long tracts of code (or data, or output, or indeed anything else you want to post) in <spoiler>...</spoiler> or <readmore>...</readmore> tags: but don't overdo it though — presenting too little can be just as bad as presenting too much.

      "i forgot to mention - if i don't explicitly use "my (variable)" at the beginning of the code, i'm continuously told ... that it isn't explicitly defined at all, ..."

      I suspect that was related to your indentation/nesting problem:

      if (COND1) { my $var = ... } if (COND2) { # $var OUT-OF-SCOPE here }

      That code is really:

      if (COND1) { my $var = ... } if (COND2) { # $var OUT-OF-SCOPE here }

      But the code you needed/intended was:

      if (COND1) { my $var = ... if (COND2) { # $var IN SCOPE here } }

      If you're referring to something else, you'll need to post example code.

      — Ken

      ... i keep getting HASH(x) in place of the names i'm putting in ...
      ... Phil HASH(0x6453d0) ...

      That's because:

      1. You still have statements like
            $son_father{$name1} = {$add_dad};
        which add an (incomplete) hash reference instead of a name; and
      2. You still do not have a
            use warnings;
        statement at the start of your script to enable Perl to tell you (update: or at least give you a hint) about stuff like this.

      I would add that you're also still using a weird indenting scheme that makes the inherent structure of your code very hard for me to follow; I hope it's easier for you, but I suspect not.


      Give a man a fish:  <%-{-{-{-<

        oh, shoot. i added "warnings" and "diagnostics" after posting this comment, my apologies... i'm still learning what's easiest for me in formatting, but i think i understand what everyone is saying re: indentations.

        an incomplete hash reference... i think i get it. i'll make sure to ask my professor about this. hashes are still brand new to me, so i'm getting easily frustrated when i really shouldn't be. thank you!

      If you indent this block

      if ($choice eq 'a'){ print "Enter a male name: "; chomp (my $name1 = ucfirst lc <STDIN>); } if (exists $son_father{$name1}) { print "Duplicate name -- try again!\n"; } else { print "Add a father: "; chomp (my $add_dad = ucfirst lc <STDIN>); $son_father{$name1} = {$add_dad}; next; }

      correctly you can see it is 2 blocks not 1

      if ($choice eq 'a'){ print "Enter a male name: "; chomp (my $name1 = ucfirst lc <STDIN>); } if (exists $son_father{$name1}) { print "Duplicate name -- try again!\n"; } else { print "Add a father: "; chomp (my $add_dad = ucfirst lc <STDIN>); $son_father{$name1} = {$add_dad}; next; }

      What you want is probably

      if ($choice eq 'a'){ print "Enter a male name: "; chomp (my $name = ucfirst lc <STDIN>); if (exists $son_father{$name}) { print "Duplicate name $name -- try again!\n"; } else { print "Add a father: "; chomp (my $add_dad = ucfirst lc <STDIN>); $son_father{$name} = $add_dad; print "$name added with father $add_dad\n"; } next; }
      poj

        i was told this last night by a friend on twitter! but for some reason when i tried to write this, the code insisted it wasn't correct. it works when you write it the way you've done it. i wonder why it told me i was wrong. thank you!

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://1202325]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others scrutinizing the Monastery: (5)
As of 2024-04-23 23:49 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found