You have a number of issues with your code.
Looking first at your readFromFile method:
- You're passing in an array that your method is filling up with all the records it finds. What happens if there are more customers in the file than there's room for in the array? (hint:
ArrayIndexOutOfBoundsException is a thing)
- You're parsing an integer read as a string from the file. What happens if the file is corrupt and the line read is not an integer?
- The name of the file to read from is hard-coded. This should be a constant or configuration option. For the purpose of writing methods, it is best to make it a parameter.
- You're opening the file and reading from it in the method. For purposes of unit testing, you should split this into separate methods.
- In general, you should be using a
Collections class instead of an array to hold a list of objects.
- You're accessing the
Customer attributes directly in the readFromFile method. You should be using an accessor method.
Collections-based approach
Here's my proposed rewrite based on using Collections APIs:
public static List<Customer> readFromFile(String filename) throws IOException {
// set up file for reading
// br used to read from file
File inputFile = new File(filename);
FileInputStream fis = new FileInputStream(inputFile);
BufferedReader br = new BufferedReader(new InputStreamReader(fis));
List<Customer> customers = readFromStream(br);
br.close(); // end ReadFile class
return customers;
}
This uses this method to actually read the contents:
public static List<Customer> readFromStream(BufferedReader br) throws IOException {
List<Customer> customerList = new LinkedList<>();
// Subtract AND assignment operator, It subtracts right operand from the
// left operand and assign the result to left operand
boolean moreCustomers = true;
while (moreCustomers) {
try {
Customer customer = new Customer();
customer.setName(br.readLine());
String sCustNo = br.readLine();
customer.setNumber(Integer.parseInt(sCustNo));
if (customer.getNumber() == 0) {
moreCustomers = false;
}
else {
customerList.add(customer);
}
}
catch (NumberFormatException x) {
// happens if the line is not a number.
// handle this somehow, e.g. by ignoring, logging, or stopping execution
// for now, we just stop reading
moreCustomers = false;
}
}
return customerList;
}
Using a similar approach for writeToFile, we get:
static void writeToFile(Collection<Customer> customers, String filename) throws IOException {
// set up file for output
// pw used to write to file
File outputFile = new File(filename);
FileOutputStream fos = new FileOutputStream(outputFile);
PrintWriter pw = new PrintWriter(new OutputStreamWriter(fos));
writeToStream(customers, pw);
pw.flush();
pw.close();
}
static void writeToStream(Collection<Customer> customers, PrintWriter pw) throws IOException {
for (Customer customer: customers) {
pw.println(customer.getName());
pw.println(customer.getNumber());
}
pw.println(0);
pw.println(0);
}
However, we still haven't addressed your main concern. It seems you want to merge the file content with the customers in memory when you call writeToFile. I suggest that you instead introduce a new method for this purpose. This keeps the existing methods simpler:
static void syncToFile(Collection<Customer> customers, String filename) throws IOException {
// get a list of existing customers
List<Customer> customersInFile = readFromFile(filename);
// use a set to merge
Set<Customer> customersToWrite = new HashSet<>();
// first add current in-memory cutomers
customersToWrite.addAll(customers);
// then add the ones from the file. Duplicates will be ignored
customersToWrite.addAll(customersInFile);
// then save the merged set
writeToFile(customersToWrite, filename);
}
Oh... I almost forgot: The magic of using a Set to merge the file and in-memory list relies on you to implement the equals() method in the Customer class. If you overwrite equals(), you should also overwrite hashCode(). For example:
public class Customer {
@Override
public boolean equals(Object obj) {
return (obj != null) && (obj instanceof Customer) && (getNumber() == ((Customer)obj).getNumber());
}
@Override
public int hashCode() {
return getNumber()+31;
}
};
CustomerList-based approach
If you cannot use Collections APIs, the second-best would be to write your own collection type that supports the same operations, but is backed by an array (or linked list, if you have learned that). In your case, it would be a list of customers. I'll call the type CustomerList:
Analyzing our existing code, we'll need a class that implements an add method and a way to traverse the list. Ignoring Iterators, we'll accomplish the latter with a getLength and a getCustomer (by index). For the synchronization, we also need a way to check if a customer is in the list, so we'll add a contains method:
public class CustomerList {
private static final int INITIAL_SIZE = 100;
private static final int SIZE_INCREMENT = 100;
// list of customers. We're keeping it packed, so there
// should be no holes!
private Customer[] customers = new Customer[INITIAL_SIZE];
private int numberOfCustomers = 0;
/**
* Adds a new customer at end. Allows duplicates.
*
* @param newCustomer the new customer to add
* @return the updated number of customers in the list
*/
public int add(Customer newCustomer) {
if (numberOfCustomers == customers.length) {
// the current array is full, make a new one with more headroom
Customer[] newCustomerList = new Customer[customers.length+SIZE_INCREMENT];
for (int i = 0; i < customers.length; i++) {
newCustomerList[i] = customers[i];
}
// we will add the new customer at end!
newCustomerList[numberOfCustomers] = newCustomer;
// replace the customer list with the new one
customers = newCustomerList;
}
else {
customers[numberOfCustomers] = newCustomer;
}
// we've added a new customer!
numberOfCustomers++;
return numberOfCustomers;
}
/**
* @return the number of customers in this list
*/
public int getLength() {
return numberOfCustomers;
}
/**
* @param i the index of the customer to retrieve
* @return Customer at index <code>i</code> of this list (zero-based).
*/
public Customer getCustomer(int i) {
//TODO: Add boundary check of i (0 <= i < numberOfCustomers)
return customers[i];
}
/**
* Check if a customer with the same number as the one given exists in this list
* @param customer the customer to check for (will use customer.getNumber() to check against list)
* @return <code>true</code> if the customer is found. <code>false</code> otherwise.
*/
public boolean contains(Customer customer) {
for (int i = 0; i < numberOfCustomers; i++) {
if (customers[i].getNumber() == customer.getNumber()) {
return true;
}
}
// if we got here, it means we didn't find the customer
return false;
}
}
With this implemented, the rewrite of the writeToFile method is exactly the same, except we use CustomerList instead of List<Customer>:
static void writeToFile(CustomerList customers, String filename) throws IOException {
// set up file for output
// pw used to write to file
File outputFile = new File(filename);
FileOutputStream fos = new FileOutputStream(outputFile);
PrintWriter pw = new PrintWriter(new OutputStreamWriter(fos));
writeToStream(customers, pw);
pw.flush();
pw.close();
}
The writeToStream is also very similar, except since we're not using an Iterator, we have to traverse the list manually:
static void writeToStream(CustomerList customers, PrintWriter pw) throws IOException {
for (int i = 0; i < customers.getLength(); i++) {
pw.println(customers.getCustomer(i).getName());
pw.println(customers.getCustomer(i).getNumber());
}
pw.println(0);
pw.println(0);
}
Similar for readFromFile -- pretty much the same except for the list type:
public static CustomerList readFromFile(String filename) throws IOException {
// set up file for reading
// br used to read from file
File inputFile = new File(filename);
FileInputStream fis = new FileInputStream(inputFile);
BufferedReader br = new BufferedReader(new InputStreamReader(fis));
CustomerList customers = readFromStream(br);
br.close(); // end ReadFile class
return customers;
}
The readFromStream is also pretty much the same, except for the type (the methods used on CustomerList has the same signature as the ones used on List<Customer>:
public static CustomerList readFromStream(BufferedReader br) throws IOException {
CustomerList customerList = new CustomerList();
// Subtract AND assignment operator, It subtracts right operand from the
// left operand and assign the result to left operand
boolean moreCustomers = true;
while (moreCustomers) {
try {
Customer customer = new Customer();
customer.setName(br.readLine());
String sCustNo = br.readLine();
customer.setNumber(Integer.parseInt(sCustNo));
if (customer.getNumber() == 0) {
moreCustomers = false;
}
else {
customerList.add(customer);
}
}
catch (NumberFormatException x) {
// happens if the line is not a number.
// handle this somehow, e.g. by ignoring, logging, or stopping execution
// for now, we just stop reading
moreCustomers = false;
}
}
return customerList;
}
The most different method is the syncToFile, as we don't have the Set type that guarantees no duplicates, we have to manually check each time we try to insert a customer from the file:
static void syncToFile(CustomerList customers, String filename) throws IOException {
// get a list of existing customers
CustomerList customersInFile = readFromFile(filename);
// use a set to merge
CustomerList customersToWrite = new CustomerList();
// first add current in-memory customers
for (int i = 0; i < customers.getLength(); i++) {
customersToWrite.add(customers.getCustomer(i));
}
// then add the ones from the file. But skip duplicates
for (int i = 0; i < customersInFile.getLength(); i++) {
if (!customersToWrite.contains(customersInFile.getCustomer(i))) {
customersToWrite.add(customersInFile.getCustomer(i));
}
}
// then save the merged set
writeToFile(customersToWrite, filename);
}
Something to note here is that we could have optimized the add operations by having an extra constructor for CustomerList that took the new capacity, but I'll leave at least something for you to figure out ;)
