Your colleague is working on an application that analyzes statistics for vehicles of different makes and models. A business rule requirement is that each car instance in the application must have a unique manufacturing year; creating a second instance of a vehicle with the same year must fail.
The Car class created by the colleague is as follows:
Copy code snippet
Copied to ClipboardError: Could not CopyCopied to ClipboardError: Could not Copy
public class Car {
Integer year;
String make;
String model;
List<Integer> existingYears = new ArrayList<>();
public Car(Integer year, String make, String model) {
this.year = year;
this.make = make;
this.model = model;
existingYears.add(year);
checkYear(year);
}
void checkYear(Integer year) {
for (int i=0; i < existingYears.size()-1; i++ ) {
if (existingYears.get(i).equals(year)) {
throw new IllegalArgumentException("Duplicated year !");
}
}
}
}
public class Car {
Integer year;
String make;
String model;
List<Integer> existingYears = new ArrayList<>();
public Car(Integer year, String make, String model) {
this.year = year;
this.make = make;
this.model = model;
existingYears.add(year);
checkYear(year);
}
void checkYear(Integer year) {
for (int i=0; i < existingYears.size()-1; i++ ) {
if (existingYears.get(i).equals(year)) {
throw new IllegalArgumentException("Duplicated year !");
}
}
}
}
You wonder if the business constraint is properly implemented and decide to test the code using the following code fragment:
new Car(2018, "Honda", "Civic"); // line n1
new Car(2021, "Hyundai", "Accent"); // line n2
new Car(2018, "Ford", "Expedition"); // line n3
Which statement is true? Choose one.
A. The test code runs without throwing an exception.
B. Line n1 will cause IllegalArgumentException.
C. Line n1 will cause IndexOutOfBoundsException.
D. Line n3 will cause IllegalArgumentException.
E. Line n2 will cause IndexOutOfBoundsException.
Answer. This example is perhaps not code you’d be proud of, but you didn’t write it, so there’s no need for embarrassment here.
Look at the code presented and notice that it seems the intention is roughly as follows:
◉ For each call to the constructor, add the proposed list to a list called existingYears.
◉ Call the method checkYear, which performs two things. First it determines if that year has been used before and, if it has, it throws an IllegalArgumentException.
The root problem here is that the existingYears list is created as an instance member of a Car object, and therefore every Car object has a List of its own. Consequently, the years in which other Car objects were created will not be found in each List. The code does not work as intended and will not prevent two separate instances of Car from having the same year value.
If instead the existingYears field carried the static modifier, only one instance List would exist in the program. In that case, the comparison would check the current car’s year with those seen before and line n3 would throw an IllegalArgumentException. That change would make option D correct. But since that wasn’t done, option D is incorrect.
You might notice that the current car’s year is added to the list before the check is run. If you fail to pay close attention to the control of the loop that searches through the list, you might expect that an exception would be thrown for every new car, based on an expected collision with its own year. That would lead you to believe that the code would throw an IllegalArgumentException for any and all attempts to instantiate a Car. In that situation, the first exception would presumably prevent further execution, and you would expect an exception at line n1. However, a more thorough investigation shows that the loop stops before the last item is checked, which is why option B is incorrect.
Notice the following test in the loop:
i < existingYears.size()-1
Because the size value is reduced by one, the code would ignore the last item in the list. That item will be the current car’s year, so the feared collision would not occur. Further, because existingYears is an instance field, the list’s size in the checkYear method is always 1, and the body of the loop never executes at all.
Two of the options suggest that an IndexOutOfBoundsException might be thrown. However, as just noted, the size of the list is always 1 when the checkYear method executes, and the body of the loop will never execute. No code outside that loop attempts to access data from the list using an index; therefore, no code that could possibly throw an IndexOutOfBoundsException will ever be executed. From this you can conclude that options C and E are incorrect.
From the observations above, you can conclude that no exceptions are thrown, and option A must be correct.
Now that the answer has been determined, here are some of the least pleasant aspects of the code—besides the fact that it doesn’t work.
◉ Even though the code for avoiding a collision with the current car’s year is functionally correct, if an attempt is made to add a car that has a duplicate year, existingYears will contain the same year twice. While this would not cause errors in the code shown so far, unexpected behaviors such as this are confusing to programmers trying to make sense of things later, both when they are reading code and when they are debugging. It’s also common for this sort of situation to lead to bugs later when the peculiarity is not thoroughly understood by all. It would be far better to add the year only after it has been approved.
◉ Creating any object with nonprivate mutable fields is generally unwise. It allows changes to be made by any code in the same package. If such changes are made without understanding the implications and, thereby violating some consistency rule pertaining to the data, it’s easy to cause bugs that are hard to track down, especially because such a large amount of code (the entire package or more) might have been responsible for the change.
◉ Checking to see if a Collection contains a given object should be done using the contains method. It’s immediately and unambiguously clear what this method does. By contrast, writing a loop describing how to perform the action leaves the programmer who is reading the code with the responsibility for working out what the code is intended to achieve. Generally speaking, it’s preferable that code have a more declarative nature and a less imperative one. Of course, to be able to use the contains method, you must first ensure that the current car’s year is not added before validation.
◉ Iterating lists using the traditional C-style for loop to control an index for accessing list elements should be avoided. Some lists in Java’s collections (typically those with array backing) iterate tolerably efficiently this way, but if the implementation changes to use a different structure (such as a LinkedList), the resulting efficiency will be relatively or exceedingly poor. If possible, give any data structure the chance to perform its own iteration, since the programmer who created that structure should know best how to iterate the elements efficiently. Instead of using an index-counting loop such as the C-style loop, use Java’s enhanced for loop or extract an Iterator (or perhaps ListIterator, if the situation calls for it) and use that.
Conclusion. The correct answer is option A.
Source: oracle.com
Posts
0 comments:
Post a Comment