Removing an entry in a map within another map?

[GlenRogers]

I have a TreeMap<String, Customer> (Customer is a class I have created).
Customer objects hold a HashMap<String, Repair> (Repair is a class I created).

I wrote a method to find a given Repair, this is it:

public void processFindRef()
{
    if(ref.length() == 6) // ref is the string we want to find, it should be 6 characters long
    {
        for(Customer c : aMap.values()) // loop through the treemap
        {
            bMap = c.getRepairs();
            for(String s : bMap.keySet()) // loop through the hashmap
            {
                if(s.equals(ref)) // what to do if ref is found as a key in bmap(hashmap)
                {
                     gui.results.setText(c.getName() + (": ") + c.toString() + newline + ref + newline + bMap.get(ref).toString());
                }
             }
         } 
     }
     else
     {
         JOptionPane.showMessageDialog(frame, "Job number does not exist", "Error",
                                       JOptionPane.WARNING_MESSAGE);
     }
 }

Now I need to write a method that removes the Repair from the hashmap(repairs) that a Customer object holds. So far I have this:

 public void processDeleteRef()
    {
       processFindRef(); 
       JOptionPane.showConfirmDialog(frame, "Do you really want to remove this job?",
                                             "Warning", JOptionPane.YES_NO_OPTION);

    }

Then i am llost as to what comes next!

Any help apprecciated!
Thanks

[JamesCherrill]

If you have the key then it's just bMap.remove(ref) - or did I misunderstand your question?

[JamesCherrill]

public void processFindRef()

This method obviously relies on sharing input and output variables to work. This is bad practice because (1) that creates undocumented dependencies that make maintenance/enhancement dangerous and (b) it prevents their re-use elsewhere.
Much better to make the inputs and outputs explicit, as in
public Customer findCusForRepair(String ref) {... or just
public Repair findRepair(String ref) {... .. or both!

[GlenRogers]

Sorry for my lack of knowledge here, but you just lost me James!

[JamesCherrill]

OK, my fault, sorry.
To delete the entry you need to have a reference to the Customer's repair map, because that's where you need to remove it from. Then removing it is just a call to the remove method passing the ref (key) of the entry to remove.
That's why I suggested moving some of your existing code into a little method that finds and returns the Customer who owns a given Repair (identified by the repair ref). Once you have the Customer then you can get the repair map, and that's that.

[GlenRogers]

I'm not really geing this! If I were to try and put the removal into my processFindRef() method , where would i put it? Does it make any difference that my metho first reads from file, the after it does its thing writes back to file?
Here my method.

private void processFindRef()
    {
        processReset3();
        String ref = gui.refText.getText().toUpperCase();
        Map<String, Customer> aMap = new TreeMap<String, Customer>();
        Map<String, Repair> bMap;

         try
         {
             File file = new File("reboot_customer_details.txt");  
             FileInputStream f = new FileInputStream(file);  
             ObjectInputStream s = new ObjectInputStream(f);  
             aMap = (TreeMap<String, Customer>)s.readObject();
             s.close();

         }
         catch(Exception ex)
         {
             gui.results.setText("Error: " + ex);
         }
         if(ref.length() == 6)
         {
             for(Customer c : aMap.values())
             {
                 bMap = c.getRepairs();
                 for(String s : bMap.keySet())
                 {
                     if(s.equals(ref))
                     {
                         gui.results.setText(c.getName() + (": ") + c.toString() + newline + ref + newline + bMap.get(ref).toString());
                     }
                 }
              }
         }
         else
         {
         JOptionPane.showMessageDialog(frame, "Job number does not exist", "Error",
                                       JOptionPane.WARNING_MESSAGE);
         }
         try
         {

             File file = new File("reboot_customer_details.txt");  
             FileOutputStream f = new FileOutputStream(file);  
             ObjectOutputStream s = new ObjectOutputStream(f);          
             s.writeUnshared(aMap);
             s.close();
         }
         catch(Exception ex)
         {
             gui.results.setText("Error: " + ex);
         }

    }

[JamesCherrill]

I'm not sure I can say much more without a full understanding of how you have structured the whole application. Sorry.

[GlenRogers]

This is my first application outside the small ones I have done for assignment so I'm sure it's structured badly.
Thanks for your input anyway!
Glen..

[JamesCherrill]

The best advice I can give you now is to keep breaking up your code into smaller methods; 10-20 lines of code is plenty for one method.
Eg the code above can be (should be) split into at least 4 methods - read from file, write back to file, find a Repair, display a Repair in the GUI. Your processFindRef will then just be 4 method calls, and it will be totally obvious what it does.
That way each method is easy to understand, and easy to re-use in other parts of the program when you want to do similar but different things.

[GlenRogers]

I have 3 or more methods that read from file, do their thing, then write back to file. The reading and writing is the same for every method. Should I have 2 small methods, 1 for reading, one for writing then just call them where needed? Is that what you mean?

[JamesCherrill]

Yes. Exactly that.
Any time you repeat code you should think about putting that into a small method and calling it from wherever it's needed.

[GlenRogers]

Sounds sensible. I'll get onto that, I do have some large methods!

[GlenRogers]

I have split a load of methods into smaller ones, my code is easier to read now, thanks james!.

I still can not delete a repair though.
Here are the methods I'm using for it.

Method to read from file.

     private void readFile()
     {
         try
         {
             File file = new File("reboot_customer_details.txt");  
             FileInputStream f = new FileInputStream(file);  
             ObjectInputStream s = new ObjectInputStream(f);  
             customerDetails = (TreeMap<String, Customer>)s.readObject();
             s.close();
          }
         catch(Exception ex)
         {
             gui.results.setText("Error: " + ex);
         }
     }

Method to write to file.

     private void writeFile()
     {
         try
         {

             File file = new File("reboot_customer_details.txt");  
             FileOutputStream f = new FileOutputStream(file);  
             ObjectOutputStream s = new ObjectOutputStream(f);          
             s.writeUnshared(customerDetails);
             s.close();
         }
         catch(Exception ex)
         {
             gui.results.setText("Error: " + ex);
         }
     }

Method to return the Customer for a given Repair

     private Customer getCustomerRepair()
     {
         String ref = gui.refText.getText().toUpperCase();
         Map<String, Repair> bMap;
         Customer cus = new Customer("aName", "aStreet", "aTown", "aPost", "aPhone");
         for(Customer c : customerDetails.values())
         {
             bMap = c.getRepairs();
             for(String s : bMap.keySet())
             {
                 if(s.equals(ref))
                 {
                     cus = c;
                     gui.results.setText(c.getName().toUpperCase() + newline + c.toString().toUpperCase() + newline + newline +
                                         ref + newline + newline + bMap.get(ref).toString());
                 }
             }
         }
         return cus;
     }

Method to return a Map<String,Repair> of repair for a Customer

    private Map getRepairs()
    {
        Repair repair;
       Customer customer = (Customer) customerDetails.get(gui.nameText.getText().toUpperCase());
       gui.results.setText(customer.getName().toUpperCase() + newline
                            + customer.toString().toUpperCase() + newline + newline);
       for (String s : customer.repairs.keySet()) 
       {
           repair = customer.repairs.get(s);
           gui.results.append(s.toString().toUpperCase() + ": " + newline +
           repair.toString().toUpperCase() + newline);
       }
       return customer.repairs;
    }
 }

Method to Display in the given repair (this woks)

    private String processFindRef()
    {
        processReset3();
        String ref = gui.refText.getText().toUpperCase();

        readFile();

         if(ref.length() == 6)
         {
             getCustomerRepair();
         }
         else
         {
         JOptionPane.showMessageDialog(frame, "Job number does not exist", "Error",
                                       JOptionPane.WARNING_MESSAGE);
         }
         return ref;
    }

Method to remove the given repair (doews not work)

    private void processDeleteRef()
    {
        readFile();
        getCustomerRepair().getRepairs().remove(processFindRef());
        writeFile();
    }

Any advice is appreciated!
Thanks...

[JamesCherrill]

getCustomerRepair().getRepairs().remove(processFindRef());

I don't knw what processFindRef() does here, but all you need is the actual ref (6 char String, I believe)
getCustomerRepair().getRepairs().remove(ref);

[GlenRogers]

I don't know what I was thinking, but thats it sorted now!
Thank you
Glen..