I'd like to refactor my extractSimplePromptName test so it does the same but it's "prettier", hence less code. As far as looking at it, it seems like it could be refactored so it does the same thing, but it's just less code. I'm just fairly new to programming so having a hard time seeing where I should make the changes.

Guessing it has something to do with putting some stuff out of the first if sentence or perhaps just removing something that doesn't do anything.

I just ain't that certain, and I'd really like some help so I can get better at doing decent code.

5 Answers
5

Often, when performing text manipulation, Regular Expressions can do the work for you. Your solution is essentially manually building a text parser and stripper, when a regular expression will do the work in a much more concise way.

The regular expression would be to remove all text up to the last \ or ; character, which would be written as:

Your tests are named horribly; make sure they convey what is being tested.

Your code makes it look as if you're testing some helper method inside your tests. What exactly is going on? Make sure that you are actually testing your application and aren't extracting all the effort into a "helper" method in your tests.

That being said I'll assume that your extractSimplePromptName method is actually the application. If it isn't then you are doing it wrong because you should use input in your tests that represents input which you will actually receive in the live application.

Do the parameter checking separately to make it clear and to reduce indentation:

if(promptNameOrg == null) {
return "";
}

I don't see any benefit to using a List<String> over an array. I would just stick to the array so you don't have to wrap it into another collection:

See what I did there? I magically reduces nearly all your indentation by one step (while keeping a correct indentation). This is done by switching the condition for the if statement and returning inside that if.

I'll do that again with if (promptNameOrg.contains(";") || promptNameOrg.contains("\\")) { (just be sure that you reverse it correctly when you do this with an if-statement that contains || or &&)
`

Your test procedures aren't named very well. The name of the procedure should tell us what we're testing for. Labeling them as 1,2,3 means we have to look at the test code to figure out which test failed. Make the names clear and meaningful, even if they're abnormally long. That's okay for tests.

It doesn't look like you've covered all of the possible test cases. It looks like it's possible for your input to have two consecutive slashes, but you don't test that possibility. You only test if there's one.