1

So I'm making a few classes that handle a collection of DVD Objects. My add and remove methods are supposed to do this:

add – this method is used to add a new DVD. It should have five parameters that represent the title, category, running time, year, and price of a DVD. If the title is already in the DVD collection, there is no need to add or change anything.Otherwise, the DVD is added to the collection. It returns the DVD entry if it is already in the DVD collection, or returns null if a new one is added.

remove – this method should have a title as the parameter. It should remove the DVD from the collection if the title is found. It returns the DVD entry which was removed, or returns null if the title is not found.

My methods currently work only for the first object in my text file but when I type in another object further down the file, It just returns null.

My text file contains the following 6 objects. Adam Documentary 78 minutes 2012 7.99 Choo Choo Documentary 60 minutes 2006 11.99 Good Morning America Documentary 80 minutes 2010 9.99 Life is Beautiful Drama 125 minutes 1999 15.99 Morning Bird Comic 150 minutes 2008 17.99 Mystic River Mystery 130 minutes 2002 24.99

public DVD add(String titlez, String categoryz, String runTimez, String yearz, String pricez) {
            Iterator<DVD> it = arraylist.iterator();
            DVD dvd = it.next();
            if(dvd.getTitle().equals(titlez)){
            return dvd;
        }
        else{
            DVD dvd1 = new DVD (titlez, categoryz, runTimez, yearz, pricez);
            arraylist.add(dvd1);

            return null;
        }
    }

    @Override
    public DVD remove(String title) {
        Iterator<DVD> it = arraylist.iterator();
        DVD dvd =  it.next();
        if(dvd.getTitle().equals(title)){
            arraylist.remove(dvd);
            return dvd;
        } else {
            return null;
        }

    }
3
  • Please see How to create a Minimal, Complete, and Verifiable example. Your code does not compile in its current form. Commented Oct 31, 2017 at 19:08
  • First question. What do you want your add method to do? Forget the code, please describe what you would like it to do. Commented Oct 31, 2017 at 19:11
  • Its literally posted right next a the method name and a hyphen Commented Oct 31, 2017 at 19:47

3 Answers 3

3

You are not looping on entire list try to use this instead:

Iterator<DVD> it = arraylist.iterator();    
while(it.hasNext()) {
    DVD dvd =  it.next();
    if(dvd.getTitle().equals(title)){
        arraylist.remove(dvd);
        return dvd;
    }
}
return null;
Sign up to request clarification or add additional context in comments.

2 Comments

Thank you, so I thought about using a for loop but was having some difficulty, however while didn't come to mind.
You are welcome, you can also use for each as in Ted Hopp's answer.
1

Your add method is not iterating through the list; it's just testing the first element. (It would also throw an exception if your list was empty.) Try this instead, which iterates through the entire list before deciding that the title is not present. (I'm using the enhanced for loop syntax instead of a traditional for loop.)

public DVD add(String titlez, String categoryz, String runTimez, String yearz, String pricez) {
    for (DVD dvd : arrayList) {
        if(dvd.getTitle().equals(titlez)){
            return dvd;
        }
    }
    DVD dvd1 = new DVD (titlez, categoryz, runTimez, yearz, pricez);
    arraylist.add(dvd1);

    return null;
}

Your remove method has a similar problem. Use this instead:

public DVD remove(String title) {
    for (DVD dvd : arrayList) {
        if (dvd.getTitle().equals(title)) {
            arrayList.remove(dvd);
            return dvd;
        }
    }
    return null;
}

Note that this is style is a bit dangerous. Normally, you shouldn't modify a list while iterating through it. If the iteration continued, you'd get a ConcurrentModificationException thrown. However, since you also stop iterating immediately upon modifying the list, it should be okay. There are two ways to avoid an exception and still modify the list. (1) use a ListIterator instead of an Iterator, because ListIterator has it's own remove() method you can use. You'd have to go back to a traditional for loop syntax. (2) defer the removal until iteration is done, like this:

public DVD remove(String title) {
    DVD toRemove = null;
    for (DVD dvd : arrayList) {
        if (dvd.getTitle().equals(title)) {
            toRemove = dvd;
            break;
        }
    }
    if (toRemove != null) {
        arrayList.remove(toRemove);
    }
    return toRemove;
}

Comments

0

As someone has already "answered", you are not iterating through the whole list.

I have refactored your code to make the add and remove methods cleaner. I have created a POJO (plain old java object) DVD, and a DVDService which has a DVDStore.

The DVDStore stores a DVD using the titlez as its "key". The add and remove methods use an exists method to check if the DVD key is in the DVDStore.

I return true of false for add or remove. I try and add a new DVD "film1" twice, and then remove twice.

The output from running is here:

film1 has been added = true
film1 has been added = false
film1 has been removed = true
film1 has been removed = false

I have removed the iterator you are using, and search the HashMap for the "titlez" key. I also return a "simple boolean" for add and remove. Has add or remove been successful (true or false).

This makes add and remove easy to understand, and maintain.

import java.util.HashMap;
import java.util.Map;

class DVD {
    private String titlez;
    private String categoryz;
    private String runTimez;
    private String yearz;
    private String price;

    /**
     * DVD constructor
     *
     * @param titlez - title
     * @param categoryz - category
     * @param runTimez - length of file
     * @param yearz - year made
     * @param price - price
     */
     DVD(String titlez, String categoryz, String runTimez, String yearz, String price) {
        this.titlez = titlez;
        this.categoryz = categoryz;
        this.runTimez = runTimez;
        this.yearz = yearz;
        this.price = price;
    }

    /**
     * get DVD titlez
     * @return - DVD titlez
     */
    String getTitlez() {
        return titlez;
    }
}


public class DVDService {
    private Map<String, DVD> dvdStore; // DVD store - use DVD titlez to "look     up" DVD

    /**
     * Convenience method for checking if a title exists in our DVD Store
     * @param titlez - DVD title
     * @return - true if exists in DVD store
     */
     private boolean exists(String titlez) {
        return dvdStore.containsKey(titlez);
     }


    /**
     * Add a DVD to the DVD store
     * @param dvd - DVD to be added
     * @return - true if DVD added
     */
       private boolean add(DVD dvd) {
           if (dvdStore == null) {  // if DVD store is null - create it
               dvdStore = new HashMap<>();
           }

           // if title does NOT exist - add it to the DVD store and return true
           if (!exists(dvd.getTitlez())) {
               dvdStore.put(dvd.getTitlez(), dvd);
               return true;
           }

           return false; // title already exists
        }


    /**
     * Remove DVD from DVD store
     * @param dvd - DVD to be removed
     * @return - true if DVD removed
     */
    private boolean remove(DVD dvd) {
        if (exists(dvd.getTitlez())) {
            dvdStore.remove(dvd.getTitlez());
            return true;
        }

        return false;
    }

    public static void main(String[] args) {
        DVD dvd = new DVD("film1", "Mystery", "2 hours", "1971", "2.00");

        DVDService dvdService = new DVDService();

        /**
         * Add a DVD = true
         * Add again = false as it exists in the DVD store
         *
         * Remove DVD = true as it exists
         * Remove DVD = false as it no longer exists
         */

        System.out.printf("%s has been added = %s\n", dvd.getTitlez(), dvdService.add(dvd));

        System.out.printf("%s has been added = %s\n", dvd.getTitlez(), dvdService.add(dvd));

        System.out.printf("%s has been removed = %s\n", dvd.getTitlez(), dvdService.remove(dvd));

        System.out.printf("%s has been removed = %s", dvd.getTitlez(), dvdService.remove(dvd));
    }
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.