# Not sure how to check my solution

This is a assignment question from school: Write a method called licencePlate that takes an array of objectionable words and returns a random licence plate that does not have any of those words in it. Your plate should consist of four letters a space then three numbers.

``````import java.util.*;

class MethodAssign5{
char[] lchars={'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z'};
char[] nchars={'0','1','2','3','4','5','6','7','8','9'};
boolean contain = true;
Random generator = new Random();
do{
for(int i=0;i<4;i++){
}
for(int i=0;i<3;i++){
}
contain = false;
for(String s:a){
boolean same = true;
char[] wchars = s.toCharArray();
for(int i=0;i<4;i++){
same = false;
}
}
if(same==true){
contain = false;
}
}
}while(contain==true);
return ans;
}
public static void main(String[]args){
String[] words ={"HAHA","GORD"};
}
}
``````

As you can see, I have no idea if my solution is right. It's such a low probability. What do you think about my solution? What should I do if this was a in-class test and has the same problem where the probability of getting the error is low. Anyone please help me I would appreciate it very much.

-

A few suggestions, and then I'll get to how to test it:

• Don't be afraid of whitespace -- your code is a little hard to read due to the lack of new lines. Also, it's fairly standard to indent method names inside of classes.
• Always use descriptive names: `String[] a` tells me nothing about what that parameter is.
• When `x` is a bool, `x == true` is equivalent to `x`.
• On an extremely picky note, `contains` seems like a more natural variable name for `contain` (grammatically anyway)
• It doesn't really matter when there's only 4 characters (so `4 * A.length` runs), but when doing a linear comparison, you should typically bail out of it with `break` or `continue`
• Also, you might want to look into `indexOf`, `contains` and `equalsIgnoreCase`
• Obviously this is just a little homework assignment, so it would be a bit overkill, but I would put the license plate generation in its own class in a real application.
• I would also pass the banned words as a parameter to the constructor instead of to a method -- that allows your instance to carry around the words without any farther down consumers having to know what the words are
• Imagine that you make a machine that's used in the DMV. Imagine that this machine has a "generate license plate" button. When pressed, a screen simply shows the plate.
• Passing the words to the method is like having to key in the banned words every time the button is pressed -- the DMV employee shouldn't be responsible for knowing/controlling those words
• Storing the words inside of an instance (whether set with a `setBannedWords` method or passed to the constructor) is like configuring the machine at creation. The DMV employee no longer has to know what the words are, or be responsible for entering them. Years could go by before an employee realized "Oh my, I just noticed that the machine has never output a cuss word!"
• For completeness: A third option would be to not filter the words -- it would then be up to the employee to go "Hrmmm, 'FART 743' is probably a bad license plate."

Ok, so does this work? Probably. It looks correct to my rusty-with-Java-eye anyway.

The easiest way to test it would probably be to whittle down the set of possible characters from A-Z to A-D and then make ABC a bad word. After a lot of test runs, you could be fairly certain that it was throwing out any `ABCX` or `XABC`.

This just made me realize: did your professor specify that the bad words would always be four characters? If not, your code is wrong.

So how could this be tested more properly-ish? Unfortunately it requires that the code become a lot longer. Your professor would probably think you got a bit carried away if you truly made this testable. Despite a fairly simple requirement, to test this with granularity would require breaking it down into quite a few pieces.

I can code this up if you want, but for the sake of brevity and time, I'll just describe it here.

The main problem with testing this is that all of the concerns are mixed together. You have a few concerns:

• Generating the text part
• Generating the number part

Note that separating this concerns allows you to test them independently.

"Does my class properly generate and filter license plates?" becomes "Does class X properly generate license plates? Does class Y properly filter license plates?"

The second option is easier to test because it allows you to pass in a license plate generator that is rigged to generate bad words.

The text and number generation should arguably be separated. That would allow for easily reusing generated numbers when a bad word is in the word part, but other than that, the added complexity would probably not be worth it.

One option would be to use an interface:

``````public interface LicensePlateGenerator {
}
``````

You could then have your classes implement it:

``````public class LicensePlateGeneratorRandom implements LicensePlateGenerator {

private final lchar[] = {'A', 'B', 'C', ...};

private final nchar[] = {'0', '1', '2', ...};

//Randomly grab 4 lchars
//Randomly grab 3 nchars

}

}

this.gen = generator;

}

//It might be a good idea to test how many plates have already been generated
//An infinite loop of generating could happen depending on the underlying generator
//and what kind of badWords are defined

String lp;

do {

return lp;

}

//Return false if lp contains any badWords
//and true otherwise
}

}
``````

Note how this is transparent to any consuming class. A consuming class doesn't need to know whether it has a `LicensePlateGeneratorRandom` or `LicensePlateGeneratorFiltered`; it just needs to know it has a LicensePlateGenerator.

Note that this allows you to test very easily. Testing your random generator means just checking a few outputs. Checking your filter could be done by rigging a generator:

(I would probably define this as an anonymous class in actual testing code, but my Java is too rusty to remember the syntax for that :p)

``````public class LicensePlateGeneratorFake implements LicensePlateGenerator {

private final List<String> words;
private Iterator<String> currentPos;

this.words = words;
currentPos = this.words.iterator();
}

if (!currentPos.hasNext()) {
//Something should happen here...
//could always just loop back around, or it
//might also be worth considering having the interface
//declare a certain exception as being possible in this method.
//(That would also allow a way out of the infinite loop in the
//(filter generator)
//(that could be the base class, and then a LicensePlateGeneratorInfiniteLoopException
//could be thrown in the filter class)
}
//Obviously the end of the iterator should be checked, but I'm lazy
return currentPos.next() + " 123";
}

}
``````

You then configure this fake generator to generate a certain stream of words ("ABCD", "DCBA", etc). Then, you pass this fake generator to the filter, and give the filter a list of words you know that the fake generator will generate.

If you tell the fake generator to generate "ABCD" and you tell the filter to reject "ABCD", you can then test success by whether or not the filter generator returns "ABCD" in any of the generations.

An alternative approach would be to have the filter not be a generator. Instead, your consuming code would be responsible for configuring its own filters and then generating plates until a suitable one is found.

Both designs have fairly strong pros and cons.

(Note: I completely bastardized the class names in my examples. In a real application, namespaces should be used. The code formatting on here makes namespaces a bit clumsy though, so I didn't use them.)

-
Holy, it took me so long to read all of your writings... I thank you very much for your efforts. By the way, for my question bad words would always be four characters. This is grade 12 computer science, that's why rules are simple :) –  cook cook Oct 19 '12 at 1:46
Crikey thats comprehensive :) –  dreza Oct 19 '12 at 7:57
+42 great, comprehensive answer. –  David Harkness Oct 20 '12 at 4:42

Some small notes (actually the last few are more or less small comments to Corbin's great answer):

1. The `licensePlate` method should validate its input parameters. Does it make sense to call it with `null`? If not, check it and throw a `NullPointerException`. (Effective Java, Second Edition, Item 38: Check parameters for validity)

2. Method names should be verbs, for example, `licensePlate` should be `generateLicensePlate`, while class names shouldn't be verbs. (Check Code Conventions for the Java Programming Language, Naming Conventions and Clean Code, page 25.)

3. The code contains lots of magic numbers. Using named constants instead of numbers would make the code more readable and less fragile. If you have to modify their value (for example, you need three-letter, three-digit plates instead of four-letter, three-digit plates) it's easy to forget to do it everywhere.

4. An endless `while` loop with a `return` seems more natural and easier to read to me than the `do-while` loop:

``````while (true) {
}
}
``````
5. For the reasons above I'd prefix the classes: `RandomLicensePlateGenerator`, `FilteredLicensePlateGenerator` etc.

6. The `LicensePlateGeneratorFake` class could be omitted if you use a mock library like EasyMock.

-
The assignment requires the name of the method to be `licensePlate`. Why professors think teaching their students bad style is beyond me. –  Eva Jan 17 at 2:34

While Corbin's answer is great (and I'd cetainly recommend accepting it over this), it buries the key point: your method does two things. This makes testing, understanding, and maintaince harder.

You are supposed to a) generate a licence number, and b) filter out unacceptable ones.

This should properly be split into at least 3 function, 1 to generate, 1 to reject, and 1 that calls the other 2 and only returns the result of the first if not rejected.

At that point you should see what needs testing, what can be tested, and how to do your test.

As for your final question: you fiddle with the process to make the error both frequent and repeatable.

-

1. The Random class is generally made a static field to avoid creating it multiple times.
2. The letter and number character arrays can be created using a simple method

``````private char[] getCharsInRange(final char lowerBound, final int size) {
final char[] result = new char[size];

for (int i = 0; i < size; ++i) {
result[i] = (char)(lowerBound + i);
}

return result;
}
``````

Names should be better, but I'm feeling lazy.

-