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 a license plate
- Generating the text part
- Generating the number part
- Filtering a license plate
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 { public String generateLicensePlate(); }
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', ...}; public String generateLicensePlate() { //Randomly grab 4 lchars //Randomly grab 3 nchars } } public class LicensePlateGeneratorFiltered implements LicensePlateGenerator { private final LicensePlateGenerator gen; private final List<String> badWords; public LicensePlateGeneratorFiltered(LicensePlateGenerator generator, List<String> badWords) { this.gen = generator; this.badWords = badWords; } public String generateLicensePlate() { //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 { lp = gen.generateLicensePlate(); } while (isBad(lp)); return lp; } private boolean isBad(String 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; public LicensePlateGeneratorFake(List<String> words) { this.words = words; currentPos = this.words.iterator(); } public function generateLicensePlate() { 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) //throw new LicensePlateGeneratorException("..."); //(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.)
