fix/spell_delay - #21
Conversation
This reverts commit 764dce8
… do some work around that I'm sure aren't correct.
| ConsoleOutputHandler.lineBreak(); | ||
| ConsoleOutputHandler.post("Please enter your command:"); | ||
|
|
||
| ConsoleOutputHandler console = new ConsoleOutputHandler(); |
There was a problem hiding this comment.
Can you try to make it so that you don't have to write the keyword new? And to be able to just say ConsoleOutputHandler.newOutput().something().something()
There was a problem hiding this comment.
so yeah, this is what I meant when I said I know some of this stuff I did was kind of a work around haha. I'm yet to fully get my head around static and non-static in Java. I cannot seem to do what you suggest without making the newOutput() method static. However, if I do make it static then I can no longer use ConsoleOutputhundler as a return value in it. This was the only way I could find to get it to work...
There was a problem hiding this comment.
There's nothing wrong with making the method static. That is what we have to do, to get that fluent interface. The starting method has to be static, otherwise it can't be called without making an object.
What is stopping you from using ConsoleOutputhundler as a return value?
There was a problem hiding this comment.
As soon as I make the newOutput() method static it comes up with an error on the return value saying it cannot be referenced. I think I've fixed it though by creating a new ConsoleOutputHandler inside the method.
|
|
||
| public void print(){ | ||
| try { | ||
| finalize(); |
There was a problem hiding this comment.
This was a slight misunderstanding. :D Calling the method finalize() is in most cases is very bad. You can read about it here. https://docs.oracle.com/javase/10/docs/api/java/lang/Object.html#finalize()
What I meant in general is to do any final steps, if there are any, in the process.
I dunno what that could be in this particular case. But for example, you could store all of the strings sent through post() in one variable, and on print() you can print it all at once, to sliiightly improve performance, because system.out.println can be considered a slower method than others.
Think about it first though! ;) This is a very cool coding exercise, because whenever spell() is called, it will mess up the order of the messages, if you store all of the posts and print them at once in the end.
| @@ -24,17 +31,28 @@ public static void spell(String input) { | |||
| Thread.currentThread().interrupt(); | |||
There was a problem hiding this comment.
Do you think that interrupting here is correct? What happened with the SPELL_DELAY? I can't see it being used anywhere.
There was a problem hiding this comment.
I'm a bit confused. I didn't write 'Thread.currentThread().interrupt();', it was already there.
Is the SPELL-DELAY not used when the 'spell' method is called in the New Game View?
There was a problem hiding this comment.
That is the idea. The delay needs to be used in the spell method, to delay every character that is being displayed, to simulate a typing effect. Interrupt() there is a mistake, you need to replace it with something that will achieve that typing effect.
Read a little bit about what threads are in programming, and how to do some basic operations with them. Hint: delaying execution of code.
There was a problem hiding this comment.
Ok thanks, I'll look into it. I'm just letting you know that I did not put Interrupt() there. I did not write any of that code, so i didn't want to go about deleting it.
There was a problem hiding this comment.
Yeah I can see that you didn't do it. I'm not sure why it's there though, it doesn't make much sense. So we should probably fix it.
There was a problem hiding this comment.
Hey this hasn't changed since we last reviewed it? It's there to handle the Interrupted exception if thrown by the .sleep above.
There was a problem hiding this comment.
Oh my god thanks for making me look into it. Github had folded the code for me and I totally missed the sleep.
@AdamPhilipSmith You don't have to do anything about this, it's working, I'm just blind.
@liam027 How does does interrupting the thread handle the exception? Isn't the exception happening because the thread was already interrupted?
There was a problem hiding this comment.
From my limited understanding of exception handling and threads, it goes like this: if Interrupted while sleeping the thread gets an Interrupted flag and throws the exception. Calling interrupt() on it actually clears (in this case I think toggles) the flag and it is nolonger set to "interrupted" and will just continue what it was doing.
There was a problem hiding this comment.
Your understanding is correct. But isn't that the same as just ignoring the exception? I'm not sure that anything changes, with or without that line of code.
I don't think that flag is ever checked internally, if I recall correctly. The interrupt flag is there for people to use it to handle such interruptions.
For example, if you're saving the game in a save file, and the thread gets interrupted, you should probably delete the half completed file before closing the thread, if the file is gonna be useless without it being fully finished.
Basically, the interruption mechanism is there to allow you to gracefully finish your threads halfway through, to avoid the risk of your code breaking in weird ways.
|
@AdamPhilipSmith the latest PR that got merged has a few new methods inside ConsoleOutputHandler that will need this treatment as well! |
3 participants
No description provided.