Can you explain a bit about what your code does, how it works?
–
Simon André ForsbergDec 19 '13 at 18:37

@SimonAndréForsberg My code first searches for a string between '()'. Then it replaces that string with the number of times it needs to be. Again the whole process is repeated.
–
user2369284Dec 19 '13 at 18:40

4 Answers
4

Indentation. If you are using Eclipse, please select all your code and press Ctrl + I. Your entire for-loop should be indented one step further.

Use methods. One method for inputting a String, one method for making one iteration of the "decompression".

Did you forget about how to properly use indexOf suddenly? You use indexOf as a condition for your while loop, that makes sense. But then you're using a for-loop to determine the location of the ( and ) characters you are going to work with. This makes no sense to me.

Better variable names. Thanks to your comments on your question, I managed to figure out parts of what you are doing and write a review about that. However, Daniel still has a point that your code is not readable.

Close the scanner. Your scanner is a resource that needs to be closed, call sc.close(); as soon as you are done with the scanner (that is, after you have received all the required inputs from it)

Potential problems. What if I want to encrypt the string (thisWillMessWithYourCode2), how should that string be encrypted in order to be decrypted correctly? Or what would happen if I gave your program the input 42)invalid(data? I believe your code would have a hard time with that.

Use variable names that mean something. The only variable you used that has any meaning to the reader is end. This means that unless you remember, every time you hit a variable you have to scroll up to find out what it is.

This almost looks obfuscated. (It's not, but it is painful to read.) I'm not even reviewing the logic itself. You need to use meaningful variable names or no one else will ever want to touch your code.

I have taken a while to answer this question (I started, then work got in the way...). Now that I am back to it, I see that Simon has posted a good review...

So, part 1, a review (and then later, a suggestion):

reiterating what Simon and Daniel suggest: the variable names are overly concise, and that is simply unnecessary. sc -> scanner, s -> input, st -> start ... is that too difficult? I can't even guess as to why you chose q though...

Your code uses a lot of String concatenation (stringval + someotherstring). This is inefficient. You should have a StringBuilder instance declared at the beginning, and then just copy the data in to the StringBuilder. For example the code:

Use a regular expression to avoid all tedious drudgery. Unlike the two ideas above and your original code, this should be more resilient to malformed input. Regular expressions also make it easy to support more than one digit for the repeat count. Besides those advantages, the code is very short.