I made a program that should send me two "best phones" from a list of phones in a text file, but only shows one?

98 Views Asked by At

This is a link to my java project: https://github.com/Mundanest/Java-files/tree/main/Lab%207

(This is my first time using github, as I'm not sure how else to link a java project folder so I might have messed up)(code also at bottom)

It holds 4 classes:

  • one for printing, FindBestPhone.java

  • one for creating a Phone object, Phone.java

  • one for adding phones to various lists, PhoneList.java

  • and one for parsing the text, PhoneParser.java

The main aim is to find the two best phones from a long list of phones in "phone-data.txt".

(one phone has the highest battery capacity and another with largest screen size)


I was given an example text file "phone-data-short.txt" with the best phones being the Samsung Galaxy S8 Plus and Huawei Mate 10 Pro.

My problem with it is that when I try to print the two phones (from phone-data.txt) it only prints one phone.

I'm not sure if I'm trying to print them incorrectly or if it's in the way I am storing the list after calculating the best phones.

My best guess is that the "getBestPhones" method in PhoneList.java is returning wrong or my main "FindBestPhones" is asking for the best phones incorrectly.

I've tried replacing how I return:

return Collections.unmodifiableList(allPhones);

with:

return allPhones;

in the getBestPhones method

and how I'm printing it:

System.out.println(bestPhones);

with:

for (Phone phone : bestPhones) {
    System.out.println(phone.getModel());
}

But nothing seems to work


Note: I'm not sure if it is in how the best phones are calculated since that was the skeleton code provided to me.

(the addPhone() and phoneIsDominated() methods were already there)

I am also unable to print the Phone's name in my main, but not getting two best phones is more of an issue.

I'm still new to Java so I'm probably missing something obvious.

Edit (code):

My main class:

import java.io.IOException;
import java.util.Collection;

public class FindBestPhones {
    public static String PHONES_FILE = "phone-data.txt";

    public static void main(String[] args) {
        // TODO: use the parseFile() method FROM PhoneParser.JAVA to get the phone data from the file

        // TODO: print the model names of all the best phones: getBestPhones() from PhoneList.java

        // TODO: handle I/O failures by printing an error message, that might occur in parseFile()


        
        try {
            PhoneList phoneList = PhoneParser.parseFile(PHONES_FILE);
            // Get the best phones from the phone list
            Collection<Phone> bestPhones = phoneList.getBestPhones();

            System.out.println(bestPhones); 
            
            //
            PhoneList testlist = PhoneParser.parseFile("phone-test.txt");
            Collection<Phone> testcoll = testlist.getAllPhones(); 
            System.out.println(testcoll);
            
            
        } catch (IOException e) {
            e.printStackTrace();
        }




    }
} 

My custom Phone object class:

/*
 * Phone — a model class representing a smart phone. A phone has a model name, a
 * screen size, and a battery capacity. */
public class Phone {
    private final String model;
    private final double screenSize;
    private final int batteryCapacity;
    
    public Phone(String model, double screenSize, int batteryCapacity) {
        // : Ensure the screenSize and batteryCapacity are positive
        // by throwing an IllegalArgumentException otherwise
        
        //TASK 1: 
        if (screenSize <0 || batteryCapacity <=0) {
            throw new IllegalArgumentException("Screen size and/or battery capacity must not be positive");
        }
        
        this.model = model;
        this.screenSize = screenSize;
        this.batteryCapacity = batteryCapacity;
    }

     // Gets the phone's model name.
    public String getModel() {
        return model; 
    }
    
     // Gets the phone's diagonal screen size (in inches).
    public double getScreenSize() {
        return screenSize;
    }
    
     // Gets the phone's battery capacity (in mAh).
    public int getBatteryCapacity() {
        return batteryCapacity;
    }
    
     /* Determines whether this phone "dominates" another phone, meaning
     * this phone is better in one criterion, and at least as good in the
     * other criterion. */
    
    public boolean dominates(Phone other) {
        // : implement this method
        // Task 2:
        return (this.getScreenSize() > other.getScreenSize() || 
                this.getBatteryCapacity() > other.getBatteryCapacity());

        
    }
}

My PhoneList class:

/*
  PhoneList — a model class representing a list of phones. There is 
  an addPhone method to add a phone to the list, a getAllPhones to get a list of all
  phones, and a getBestPhones method to get a list of just the ‘best’ phones.
 */


import java.util.*;

public class PhoneList {
    private final List<Phone> allPhones = new ArrayList<>();
    private final Set<Phone> bestPhones = new HashSet<>();
    
    /*
     * Adds a phone to the list.
     */
    public void addPhone(Phone phone) {
        allPhones.add(phone);
        
        // remove from bestPhones if dominated by the new phone
        Iterator<Phone> iter = bestPhones.iterator();
        while(iter.hasNext()) {
            Phone other = iter.next();
            if(phone.dominates(other)) {
                iter.remove();
            }
        } 
        
        // only add the new phone to bestPhones if it's not dominated
        if(!phoneIsDominated(phone)) {
            bestPhones.add(phone);
        }
    }
    
    /*
     * Determines whether a phone is dominated by any other phone.
     */
    public boolean phoneIsDominated(Phone phone) {
        // only need to compare with the undominated phones
        for(Phone other : bestPhones) {
            if(other.dominates(phone)) {
                return true;
            }
        }
        return false;
    }
    
    /*
     * Gets all phones. The list returned is unmodifiable.  
     */
    public List<Phone> getAllPhones() {
        // : return an unmodifiable view of the list
        // Task 3.1:
        //return allPhones;
        return Collections.unmodifiableList(allPhones);
        //return allPhones;
    }
    
    /*
     * Gets all phones which are not dominated by another phone. The
     * collection returned is unmodifiable.  
     */
    public Collection<Phone> getBestPhones() {
        // : return an unmodifiable view of the set
        //Task 3.2:
        return Collections.unmodifiableSet(bestPhones);
        //return bestPhones;
        

    }
}

My PhoneParser class:

/*
 * PhoneParser — a helper class which can parse phone data from a string, and load
 * all the phone data from a text file.
 */

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import java.util.Scanner;

public class PhoneParser {
    /*
     * Parses a phone data string, in the following format:
     * 
     *     model screenSize batteryCapacity
     *     
     * The model name is encoded with underscores instead of spaces.
     */
    public static Phone parse(String data) {
        // : parse the phone data string, and return a Phone object.
        // you may use string manipulation, a regex, or a Scanner
        //Task 4:
        try (Scanner scanner = new Scanner(data)){
            // set delimiter to split on spaces
            scanner.useDelimiter(" ");

            if (scanner.hasNext()) {
                String model = scanner.next().replace("_"," ");
                if (scanner.hasNextDouble() && scanner.hasNextInt()) {
                    double screenSize = scanner.nextDouble();
                    int batteryCapacity = scanner.nextInt();
                    return new Phone(model, screenSize, batteryCapacity);
                }

            }

        } catch (Exception e) {
            e.printStackTrace();
        }
        // Returns null if parsing fails
        return null;
    }

    /*
     * Returns a PhoneList by parsing a text file containing the phone data.
     */
    public static PhoneList parseFile(String filename) throws IOException {
        // : create a PhoneList TASK 5:
        PhoneList phoneList1 = new PhoneList();

        // : create a BufferedReader to read from the file TASK 5:
        try (BufferedReader reader = new BufferedReader(new FileReader(filename))) {

            // : for each line, parse it as a Phone and add it to the list TASK 5:
            String line;
            while ((line = reader.readLine()) != null) {
                Phone phone = parse(line);
                if (phone != null) {
                    phoneList1.addPhone(phone);
                }   
            }
        }
        return phoneList1;
    }

}

My text file

The consoles prints:

[Phone@2ed94a8b]

When I run the code.

I also made a discovery, I made a test text file and filled it with 4 test phones:

test file

and was given this in console:

[Phone@38082d64, Phone@dfd3711, Phone@42d3bd8b]

which is only picking up 3 phones instead of 4 for some reason, this is probably where my issues lie.

2

There are 2 best solutions below

0
DevilsHnd - 退した On

To me, this is rather confusing. Why couldn't the best phones have both the largest screen size and the most battery capacity as well? Which is to take precedence over one phone from another phone towards Screen Size and Battery Capacity? Shouldn't it be configurable (to some extent) to accommodate specific needs?

Try the small runnable code below (read comments in code). Be sure to play with the values for the numberOfBestPhonesToFind and precedence member variables:

public class GetBestPhoneDemo {

    /* Will Hold the max Screen size of a phone found 
       that matches desired precedence:           */
    double maxScreenSize;
    
    /* Will Hold the max Battery Capacity of a phone 
       found that matches desired precedence:     */
    int maxBatteryCapacity;
    
    // The number of "Best Phones" this app is to acquire:
    int numberOfBestPhonesToFind = 2;

    /* 0  = Both Screen Size & Battery Capacity take equal precedence. 
       1  = Screen Size takes more precedence than Battery Capacity.
       2  = Battery Capacity takes more precedence than Screen Size.  
       3  = Screen Size takes full precedence.
       4  = Battery Capacity takes full precedence.         
       5+ = Nothing is displayed.                             */
    int precedence = 0;
    
    /* Internally used counter to keep track of 
       the number of best phones acquired:   */
    int phoneCount = 0;

    /* List Interface of String to hold whatever file
       lines that are deemed to be "Best Phones":  */
    java.util.List<String> bestPhonesList = new java.util.ArrayList<>();
    
    // Will hold the currently processed "Best Phone" data Line:
    String currentBestPhone = "";
    
    
    
    // The `main()` method - Application entry point....
    public static void main(String[] args) {
        /* App started this way to avoid the need for statics
           unless we really want them:        */
        new GetBestPhoneDemo().startApp(args);
    }

    private void startApp(String[] args) {
        java.io.File dataFile = new java.io.File("Phone-Data-List.txt"); // OR "Phone-Data-Short.txt"
        while (phoneCount < numberOfBestPhonesToFind) {
            maxScreenSize = Double.MIN_VALUE;
            maxBatteryCapacity = Integer.MIN_VALUE;
            String bestPhone = "";
            
            // 'Try With Resources' used here to auto-close reader:
            try (java.util.Scanner reader = new java.util.Scanner(dataFile)) {
                String line = "";
                while (reader.hasNextLine()) {
                    line = reader.nextLine();
                    line = line.trim();
                    /* Skip blank lines (should there be any) and skip past 
                       any phone that is already in the `bestPhoneList`  */
                    if (line.isEmpty() || bestPhonesList.contains(line)) {
                        continue;
                    }
                    
                    // Split the currently read in data line...
                    String[] lineParts = line.split("\\s+");
                    
                    // Get the Battery Capacity from data line:
                    int batCap = Integer.parseInt(lineParts[lineParts.length - 1]);
                    
                    // Get the Screen Size from data line:
                    double scrnSize = Double.parseDouble(lineParts[lineParts.length - 2]);
                    
                    // Set a possible best phone based on the desired precedence:
                    if (precedence == 0 && (scrnSize >= maxScreenSize && batCap >= maxBatteryCapacity)) {
                        setBestPhone(scrnSize, batCap, line);
                    } 
                    else if (precedence == 1 && (scrnSize > maxScreenSize && batCap >= maxBatteryCapacity)) {
                        setBestPhone(scrnSize, batCap, line);
                    } 
                    else if (precedence == 2 && (scrnSize >= maxScreenSize && batCap > maxBatteryCapacity)) {
                        setBestPhone(scrnSize, batCap, line);
                    } 
                    else if (precedence == 3 && scrnSize > maxScreenSize) {
                        setBestPhone(scrnSize, batCap, line);
                    }
                    else if (precedence == 4 && (batCap > maxBatteryCapacity)) {
                        setBestPhone(scrnSize, batCap, line);
                    } 
                }
            }
            catch (java.io.FileNotFoundException ex) {
                System.err.println(ex.getMessage());
                return;
            }
            
            /* Add the determined "Best Phone" to the `bestPhonesList` 
               List providing it isn't already in there:        */
            if (!bestPhonesList.contains(currentBestPhone)) {
                bestPhonesList.add(currentBestPhone);
                phoneCount++; // Increment the phone count aquired, by 1:
            }
            
            // Read the file again to get another best phone (if needed).
        }
                
        // Display the Best Phones in Console Window:
        for (String str : bestPhonesList) {
            System.out.println(str);
        }
    }

    private void setBestPhone(double screenSize, int batteryCapacity, String bestPhone) {
        maxScreenSize = screenSize;
        maxBatteryCapacity = batteryCapacity;
        currentBestPhone = bestPhone;
    }
}

You may need to tweak it a bit but, perhaps you can get something from it.

0
Abra On

This line of your code (in method parse of class PhoneParser) will always return "false".

if (scanner.hasNextDouble() && scanner.hasNextInt()) {

You need to make it a nested if, i.e.

if (scanner.hasNextDouble()) {
    double screenSize = scanner.nextDouble();
    if (scanner.hasNextInt()) {
        int batteryCapacity = scanner.nextInt();
        return new Phone(model, screenSize, batteryCapacity);
    }
}

Initially, bestPhones (in class PhoneList) is empty. So the first line of file "phone-data.txt" must be added to bestPhones since there are no other phones to compare it to and therefore it must be a "best phone". Your code does not handle that situation.

Method dominates (in class Phone) does not implement the criteria as stated in the method comments. It should be:

return (this.getScreenSize() > other.getScreenSize()  &&  getBatteryCapacity() >= other.getBatteryCapacity()) ||
       (this.getBatteryCapacity() > other.getBatteryCapacity()  &&  getScreenSize() >= other.getScreenSize());

For the sake of completeness, here is the code of all four classes, including the changes described above.

Class Phone

public class Phone {
    private final String model;
    private final double screenSize;
    private final int batteryCapacity;

    public Phone(String model, double screenSize, int batteryCapacity) {
        // : Ensure the screenSize and batteryCapacity are positive
        // by throwing an IllegalArgumentException otherwise

        //TASK 1:
        if (screenSize <0 || batteryCapacity <=0) {
            throw new IllegalArgumentException("Screen size and/or battery capacity must not be positive");
        }
        this.model = model;
        this.screenSize = screenSize;
        this.batteryCapacity = batteryCapacity;
    }

    // Gets the phone's model name.
    public String getModel() {
        return model;
    }

    // Gets the phone's diagonal screen size (in inches).
    public double getScreenSize() {
        return screenSize;
    }

    // Gets the phone's battery capacity (in mAh).
    public int getBatteryCapacity() {
        return batteryCapacity;
    }

    /* Determines whether this phone "dominates" another phone, meaning
     * this phone is better in one criterion, and at least as good in the
     * other criterion. */
    public boolean dominates(Phone other) {
        // : implement this method
        // Task 2:
        return (this.getScreenSize() > other.getScreenSize()  &&  getBatteryCapacity() >= other.getBatteryCapacity()) ||
               (this.getBatteryCapacity() > other.getBatteryCapacity()  &&  getScreenSize() >= other.getScreenSize());
    }
}

Class PhoneLIst

import java.util.*;

public class PhoneList {
    private final List<Phone> allPhones = new ArrayList<>();
    private final Set<Phone> bestPhones = new HashSet<>();

    /*
     * Adds a phone to the list.
     */
    public void addPhone(Phone phone) {
        allPhones.add(phone);
        if (bestPhones.isEmpty()) {
            bestPhones.add(phone);
        }
        else {

            // remove from bestPhones if dominated by the new phone
            Iterator<Phone> iter = bestPhones.iterator();
            while (iter.hasNext()) {
                Phone other = iter.next();
                if (phone.dominates(other)) {
                    iter.remove();
                }
            }

            // only add the new phone to bestPhones if it's not dominated
            if (!phoneIsDominated(phone)) {
                bestPhones.add(phone);
            }
        }
    }

    /*
     * Determines whether a phone is dominated by any other phone.
     */
    public boolean phoneIsDominated(Phone phone) {
        // only need to compare with the undominated phones
        for (Phone other : bestPhones) {
            if (other.dominates(phone)) {
                return true;
            }
        }
        return false;
    }

    /*
     * Gets all phones. The list returned is unmodifiable.
     */
    public List<Phone> getAllPhones() {
        // : return an unmodifiable view of the list
        // Task 3.1:
        // return allPhones;
        return Collections.unmodifiableList(allPhones);
        // return allPhones;
    }

    /*
     * Gets all phones which are not dominated by another phone. The collection
     * returned is unmodifiable.
     */
    public Collection<Phone> getBestPhones() {
        // : return an unmodifiable view of the set
        // Task 3.2:
        return Collections.unmodifiableSet(bestPhones);
        // return bestPhones;
    }
}

Class PhoneParser

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import java.util.Scanner;

public class PhoneParser {
    /*
     * Parses a phone data string, in the following format:
     * 
     * model screenSize batteryCapacity
     * 
     * The model name is encoded with underscores instead of spaces.
     */
    public static Phone parse(String data) {
        // : parse the phone data string, and return a Phone object.
        // you may use string manipulation, a regex, or a Scanner
        // Task 4:
        try (Scanner scanner = new Scanner(data)) {
            // set delimiter to split on spaces
            scanner.useDelimiter(" ");

            if (scanner.hasNext()) {
                String model = scanner.next().replace("_", " ");
                if (scanner.hasNextDouble()) {
                    double screenSize = scanner.nextDouble();
                    if (scanner.hasNextInt()) {
                        int batteryCapacity = scanner.nextInt();
                        return new Phone(model, screenSize, batteryCapacity);
                    }
                }
            }
        }
        catch (Exception e) {
            e.printStackTrace();
        }
        // Returns null if parsing fails
        return null;
    }

    /*
     * Returns a PhoneList by parsing a text file containing the phone data.
     */
    public static PhoneList parseFile(String filename) throws IOException {
        // : create a PhoneList TASK 5:
        PhoneList phoneList = new PhoneList();

        // : create a BufferedReader to read from the file TASK 5:
        try (BufferedReader reader = new BufferedReader(new FileReader(filename))) {

            // : for each line, parse it as a Phone and add it to the list TASK 5:
            String line;
            while ((line = reader.readLine()) != null) {
                Phone phone = parse(line);
                if (phone != null) {
                    phoneList.addPhone(phone);
                }
            }
        }
        return phoneList;
    }
}

Class FindBestPhones

import java.io.IOException;
import java.util.Collection;

public class FindBestPhones {
    public static String PHONES_FILE = "phone-data.txt";

    public static void main(String[] args) {
        try {
            PhoneList phoneList = PhoneParser.parseFile(PHONES_FILE);

            // Get the best phones from the phone list
            Collection<Phone> bestPhones = phoneList.getBestPhones();

            // Print the model names of the best phones
            System.out.println("Best Phones:");
            for (Phone phone : bestPhones) {
                System.out.println(phone.getModel());
            }
        }
        catch (IOException e) {
            e.printStackTrace();
        }
    }
}

This is the output I get:

Best Phones:
Oukitel K10
Xiaomi Mi Max 2 64GB
Doogee BL12000
Asus ZenFone 3 Ultra ZU680KL (4GB RAM), 64GB