Some beginner mistakes...
The functionality of the classes should make sense logically. Instead of bringing out the terminologies, I prefer to think from a layman's interpretation of a class. Why should your PartList
know what is a CSVReader
? And why should your PartNumber
know how to read an input?
Other pointers
You also need to learn how to accurate translate the rules and requirements of your work into programming logic. For example, the identification of non-"top level" parts. Are they identified by a model level number greater than two, as written in your code? Or just when they are not equal to two, as you have written in your question? This is also where validation of values and "business requirements" come in to play, to ensure your code functions as per expectations.
Gotcha
Looking at your code again, there is a glaring bug between the for
loop below and the setList
method of PartList
, which is called in the same loop too:
for (int i = 0; i < limit; i++) {
if (row[i].contains(input)) {
// The way I'm instantiating and calling from PartList here seems off to me
PartList partList = new PartList();
partList.setList(reader, row);
}
}
setList
is actually modifying the state of your reader
object, i.e. when this method returns and there is the slim chance that the next column matches the input (I know it's not intentional, so probably bug #2?), you wouldn't be looping through the same lines of the reader
object. Instead, you will be starting from the next line which the first invocation of setList
left off. In fact, your code will also be reading in one extra line when setList
exits upon encountering the next top-level part, and the flow goes into the next iteration of the outer while
loop which calls reader.readNext()
again. Last but not least, PartList
is instantiated within the for
loop, meaning it can't be used outside at all.
Suggested solution
package com.myproject.se;
public class Part {
enum Serviceability {
SERVICEABLE("SVCBLE"), NOT_SERVICEABLE("NOT SVC"), UNDEFINED("");
private String value;
Serviceability(String value) {
this.value = value;
}
public String toString() {
return value;
}
static Serviceability fromValue(String value) {
if (value != null) {
String valueToCompare = value.toUpperCase();
for (Serviceability current : Serviceability.values()) {
if (current.value.equals(valueToCompare)) {
return current;
}
}
}
return UNDEFINED;
}
}
enum PartType {
PART("PART"), ASSEMBLY("ASSEMBLY"), UNDEFINED("");
private String value;
PartType(String value) {
this.value = value;
}
public String toString() {
return value;
}
static PartType fromValue(String value) {
if (value != null) {
String valueToCompare = value.toUpperCase();
for (PartType current : PartType.values()) {
if (current.value.equals(valueToCompare)) {
return current;
}
}
}
return UNDEFINED;
}
}
private int modelLevel;
private String partNumber;
private String name;
private Serviceability serviceability;
private int quantity;
private PartType partType;
public Part(String[] values) {
// TODO: sanity checking for desired number of columns
setModelLevel(Integer.parseInt(values[0]));
setPartNumber(values[1]);
setName(values[2]);
setServiceability(values[3]);
setQuantity(Integer.parseInt(values[4]));
setPartType(values[5]);
}
public int getModelLevel() {
return modelLevel;
}
public void setModelLevel(int modelLevel) {
this.modelLevel = modelLevel;
}
public String getPartNumber() {
return partNumber;
}
public void setPartNumber(String partNumber) {
// normalize to upper case
this.partNumber = partNumber.toUpperCase();
}
public String getName() {
return name;
}
public void setName(String name) {
// normalize to upper case
this.name = name.toUpperCase();
}
public Serviceability getServiceability() {
return serviceability;
}
public void setServiceability(Serviceability serviceability) {
this.serviceability = serviceability;
}
public void setServiceability(String serviceability) {
this.serviceability = Serviceability.fromValue(serviceability);
}
public int getQuantity() {
return quantity;
}
public void setQuantity(int quantity) {
this.quantity = quantity;
}
public PartType getPartType() {
return partType;
}
public void setPartType(PartType partType) {
this.partType = partType;
}
public void setPartType(String partType) {
this.partType = PartType.fromValue(partType);
}
public boolean isTopLevelPart() {
return modelLevel == 2;
}
public boolean hasPartNumberContains(String value) {
return partNumber.contains(value);
}
public boolean isServiceable() {
return serviceability == Serviceability.SERVICEABLE;
}
public String toString() {
return modelLevel + " | " + partNumber + " | " + name + " | " + serviceability + " | " + quantity + " | "
+ partType;
}
}
I used two Enums
to represent valid values for serviceability and part type, in case I need to compare these values often and I want to avoid any typos in doing String
comparisons. I also normalize String inputs to upper case for this exercise. By right, I can also have a PartFactory
class to take in the six String
inputs and construct a Part
for me, but I have decided to keep it simple and just implemented a constructor for this purpose. Feel free to add useful methods that is related to a Part
and will aid in your work here, for example I have the isTopLevelPart
method that will return true
if the model number is 2.
package com.myproject.se;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
public class PartList implements Iterable<Part> {
private List<Part> partList = new ArrayList<Part>();
public PartList(Part part) {
addPart(part);
}
public void addPart(Part part) {
// simple constraint
if (partList.isEmpty() && !part.isTopLevelPart()) {
throw new RuntimeException("This is not a top-level part.");
}
partList.add(part);
}
@Override
public Iterator<Part> iterator() {
return partList.iterator();
}
public Part getTopLevelPart() {
return partList.get(0);
}
public int size() {
return partList.size();
}
public List<Part> getServiceableParts() {
List<Part> result = new ArrayList<Part>();
for (Part part : partList) {
if (part.isServiceable()) {
result.add(part);
}
}
return result;
}
}
PartList
is a simple wrapper class that stores Part
objects in an ArrayList
. The only enhancement I have added is a constraint on the first element of the List
, which must be a top-level part (see above). Other than that, feel free to once again introduce methods related to what a PartList
should know, in my case here I added a simple getServiceableParts
method for illustration.
package com.myproject.se;
import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
public class RecordSupplier implements Iterable<Part>, Closeable {
private static final List<String[]> testValues = new ArrayList<String[]>();
static {
testValues.add(new String[] { "2", "ABC", "NAME 1", "", "1", "" });
testValues.add(new String[] { "3", "DEF", "NAME 2", "SVCBLE", "2", "PART" });
testValues.add(new String[] { "4", "GHI", "NAME 3", "NOT SVC", "3", "ASSEMBLY" });
testValues.add(new String[] { "2", "JKL", "NAME 4", "", "1", "" });
testValues.add(new String[] { "3", "MNO", "NAME 5", "SVCBLE", "5", "PART" });
}
@Override
public void close() throws IOException {
// do nothing
}
@Override
public Iterator<Part> iterator() {
return new Iterator<Part>() {
private Iterator<String[]> innerIterator = testValues.iterator();
@Override
public boolean hasNext() {
return innerIterator.hasNext();
}
@Override
public Part next() {
return new Part(innerIterator.next());
}
@Override
public void remove() {
throw new UnsupportedOperationException();
}
};
}
}
I needed a mock class here to simulate a CSVReader
, so here's a RecordSupplier
using the iteration pattern of providing input. What is crucial to note here is that RecordSupplier
, or more specifically its iterator
, supplies Part
objects, instead of a String
array. Generally, you will want a well-behaved input provider that passes only types that can appropriately encapsulate the raw underlying values to caller methods that need the data. The other minor benefit of wrapping your CSVReader
in such a class is that you can easily replace the actual input in the future by a database, or XML file reader etc, and all the caller methods see are still Part
objects.
package com.myproject.se;
import java.io.IOException;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Scanner;
public class Main {
public static void main(String[] args) {
@SuppressWarnings("unused")
Scanner scanner = new Scanner(System.in);
String partNumber1;
String partNumber2;
System.out.println("Enter first part number:");
// partNumber1 = scanner.next();
partNumber1 = "JKL";
System.out.println("Enter second part number:");
// partNumber2 = scanner.next();
partNumber2 = "ABC";
Map<String, PartList> result = getPartLists(partNumber1, partNumber2);
for (Entry<String, PartList> entry : result.entrySet()) {
System.out.println("Part number search term: " + entry.getKey());
Iterator<Part> iterator = entry.getValue().iterator();
while (iterator.hasNext()) {
System.out.println(iterator.next());
}
System.out.println("====================");
}
}
public static Map<String, PartList> getPartLists(String... partNumbers) {
// only searching for two parts now, easily extensible
if (partNumbers.length != 2) {
throw new RuntimeException("Only two part numbers needed.");
}
Map<String, PartList> result = new HashMap<String, PartList>();
RecordSupplier supplier = new RecordSupplier();
Iterator<Part> iterator = supplier.iterator();
PartList partList = null;
while (iterator.hasNext()) {
Part part = iterator.next();
if (part.isTopLevelPart()) {
String match = partNumberMatches(part, partNumbers);
if (match.isEmpty()) {
partList = null;
} else {
// match belongs to a new PartList
partList = new PartList(part);
// add the new PartList to result
result.put(match, partList);
}
} else if (partList != null) {
// keep adding current Part to existing PartList
partList.addPart(part);
}
}
try {
supplier.close();
} catch (IOException e) {
e.printStackTrace();
}
// returning just a List of PartList should be good enough actually
return result;
}
public static String partNumberMatches(Part part, String... partNumbers) {
for (String partNumber : partNumbers) {
if (part.hasPartNumberContains(partNumber)) {
return partNumber;
}
}
return "";
}
}
Finally, the main class. I have commented out the parts that read in user input (see the use of the Scanner
class) and hard-coded two test values. The bulk of the work is done in getPartLists
.
Map<String, PartList> result = new HashMap<String, PartList>();
RecordSupplier supplier = new RecordSupplier();
Iterator<Part> iterator = supplier.iterator();
PartList partList = null;
Create our result
object and the RecordSupplier
.
while (iterator.hasNext()) {
Part part = iterator.next();
Get a Part
object representing the current row.
if (part.isTopLevelPart()) {
String match = partNumberMatches(part, partNumbers);
If this is a top-level part, check if it matches our inputs or not (I think partNumberMatches
is pretty self-explanatory and will not go there).
if (match.isEmpty()) {
partList = null;
If there is no match, we set our partList
to null
, using this as some sort of a flag afterwards to know we are ignoring subsequent lines until the next top-level part.
} else {
// match belongs to a new PartList
partList = new PartList(part);
// add the new PartList to result
result.put(match, partList);
}
We have a match, lets set partList
as a new PartList
with this top-level part and add it to our result
.
} else if (partList != null) {
// keep adding current Part to existing PartList
partList.addPart(part);
}
}
For other non-top-level parts, we will just check if partList
is null
or not. If it isn't, it means we have just matched a top-level part to our inputs and we need to add the current part to partList
.
Implicit assumptions:
- We are only matching part number names with the second column,
Part Number Breakdown
(see partNumberMatches
and Part.hasPartNumberContains
)
- One top-level part will only match one input.
I hope this is informative enough...