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

Edited 4 Years Ago by GlenRogers

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!

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.

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);
         }

    }

Edited 4 Years Ago by 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..

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.

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?

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.

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...

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);

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

This question has already been answered. Start a new discussion instead.