Java 無限ループになる理由

Question

とあるサイトを使って、勉強のために少しプログラムを書いてみました。

こちらがサイトになります。→リンク
コンストラクタや、引数、戻り値などの勉強のためにやっております。基本的なことからあまり理解ができておらず、簡単なことかもしれませんが、よろしくお願い致します。

class Actor extends Thread{
	private int hitpoint;
	private int offence;
	private int deffence;
	private String name;
	private Actor target;
	private int deley = 1;
	
	public int getHitPoint() {
		return hitpoint;
	}
	
	public int getOffence() {
		return offence;
	}
	
	public int getDeffence() {
		return deffence;
	}
	public Actor getTarget() {
		return target;
	}
	
	public void setHitPoint(int hitpoint) {
		this.hitpoint = hitpoint;
	}
	public void setAttack(int offence) {
		this.offence = offence;
	}
	public void setDeffence(int deffence) {
		this.deffence = deffence;
	}
	
	public void setTarget(Actor target) {
		this.target = target;
	}
	public boolean isDead() {
		return hitpoint<=0;
	}
	
	public Actor(String name,int hitpoint,int offence,int deffence) {
		this.name = name;
		this.hitpoint = hitpoint;
		this.offence = offence;
		this.deffence =  deffence;
	}
	
	public void attack(Actor pActor) {
		int damage = offence - pActor.getDeffence();
		if(damage >= 0) {
			pActor.setHitPoint(pActor.getHitPoint()-damage);
			System.out.println(name+"は"+pActor.getName()+"に"+damage+"のダメージを与えた！");
			
	}else if(damage<=0) {
		System.out.println("ダメージを受けない");
	}
	
		
		}
	
	public void run() {
		while(true) {
			try {
				sleep(1 * deley);
				
			}catch(InterruptedException e) {
			}
			if(isDead())
				break;
			attack(target);
			}
		}
	}

public class MyMain {


	public static void main(String[] args) {
		
		Actor Hero = new Actor("主人公",100,25,15);
		
		Actor Enemy1 = new Actor("敵１",60,30,5);
		
				Hero.setTarget(Enemy1);
				Enemy1.setTarget(Hero);
　　　　			　　　　　Hero.start();
				Enemy1.start();

	}
}

こちらを実行すると、無限ループになってしまいます。ひとつ前に行った、

while(true) {
						
						Hero.attack(Enemy1);
						if(Enemy1.getHitPoint()  <= 0) {
						System.out.println(Enemy1.getName() +"を倒した！");
						break;
						}
						Enemy1.attack(Hero);
						if(Hero.getHitPoint() <= 0) {
						System.out.println(Hero.getName() + "は倒されてしまった。");
						break;
}
}

こちらはしっかりと動作しました。
attack(target);という表記がありますが、サイトではattack();のみでした。
このままではコンパイルが通らなかったので、勝手にtargetと入れました。これのせいでしょうか？
よろしくお願い致します。

現在、一度試したところ、実行する度に結果が変わるという現象が起きています。
実行の結果です。
敵１はThread-0に15のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
敵１はThread-0に15のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
敵１はThread-0に15のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
以下無限ループ

二回目
主人公はThread-1に20のダメージを与えた！
敵１はThread-0に15のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
敵１はThread-0に15のダメージを与えた！
敵１はThread-0に15のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
以下無限ループ

デバッグ一回目
敵１はThread-0に15のダメージを与えた！
敵１はThread-0に15のダメージを与えた！
以下無限ループ

デバッグ二回目
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
主人公はThread-1に20のダメージを与えた！
どういうことでしょうか。。。

Comments

[stdio]

hitpointにどんな値が入っているかlogで確かめましょう。

[Orlofsky]

URLは https://teratail.com/help#about-markdown の [リンク] に修正してください。

[nikkucc]

>>stdio様
hitpointには、主人公の100が入っておりました。
>>Orlofsky様
ご指摘ありがとうございます。
リンク変更いたしましたので、よろしくお願い致します。

[miyabi_pudding]

すみません、無知識で申し訳ございませんが、
while(true) {
って、Javaでは一般的な書き方なのでしょうか？
そもそも無限ループ乱発しそうな書き方だと思うのですが・・・。
中で、条件によってbreak;させているようですが、()の条件の中にも、カウンターなりで、ループの限界回数を定めた方がいいかと思いますが・・・。

[yukkuri]

> miyabi_takatsukさん
結構一般的で、スレッドとかだとよく使います。人によっては、変数に入れたりしてコントトールしていますが。ただし、気をつけないと無限ループになるのは事実です。

[miyabi_pudding]

yukkuriさん>そうだったのですね・・・！勉強になりました。
ありがとうございます！

[stdio]

isDead()がwhileを無限ループさせている原因なので、100が入っているようなら、抜けるのは無理です。
あと、attack関数の中で、hitpointの値を減らしていないように見えます。
貴方の発言から察するに常に同じ値が入っているのが原因ではないでしょうか？
ちゃんと減っているかデバックしているか確認した方が良さそうです。
またしっかりとSystem.out.println(Enemy1.getName() +"を倒した！");又はSystem.out.println(Hero.getName() + "は倒されてしまった。");のメッセージは出ていますか？
もし出ていないなら明らか、hitpointが減っていないか、逆に増えて行っている証拠です。

[nikkucc]

たくさんの回答ありがとうございます！
色々修正してみた結果、次の質問のようになっております。。
皆さんの回答をもとにもう少し手を加えてみたいと思います。
よろしければ、次の質問も回答いただけるとありがたいです。
手を加えるうちに様々な問題が発生しております。。

Answer 1

主人公・敵１｢相手を攻撃するぞ！｣

敵１死亡後
敵１｢死んだからループ抜けます｣
主人公｢生きてる限り相手を攻撃するぞ！｣

という状態です。

attackメソッドは、そう使うならフィールド変数にtargetなんていりませんよね？

Comments

[nikkucc]

回答ありがとうございます。
サイトを確認し、attackメソッドを実装したのですが、targetか何かを入れないとコンパイルエラーとなってしまい、実行をすることもできませんでした。
原因が分からず、targetを入れることでコンパイルが通ったので、大丈夫だと思いました。

[swordone]

そもそも、attackメソッドをあなたは正しく実装しているのですか？
少なくとも私はそれはありえないと考えています。

Answer 2

とりあえず原因がわかりました。

// if(isDead()) これはだめ
   if(isDead() || target.isDead()) break;

解説:
追加した || は右、左どちらかがtrueになれば実行される、という印(?)です。
右のtarget.isDead()は、相手が倒れたらtrueになります。
要するに、相手は倒れたが自分が倒れていないから繰り返し、というふうになっていたわけです。

if(isDead()) System.out.println( "やられてしまった" );
if(target.isDead()) System.out.println( "敵を倒した" );

みたいに、処理を分ければそれぞれ違うこともできます。

Comments

[nikkucc]

回答ありがとうございます！
もしよろしければ、次の質問にも回答していただけるとありがたいです。
次は、どっちも倒してしまうという現象が起きています。
ループにはとりあえずならないようになったのですが。。

Answer 3

attack関数内にあるpActor.getName()なのですが、Actorの中にgetName関数が記載されていませんね。そのせいで、Thread-1とかいう意味の分からない名前になってしまっていますね。もしかしたらそこが原因かも...

それと残りhitpointを表示できるようにしましょう。

主人公は敵１に20のダメージを与えた！
敵１のHPは残り45だ!

このようにすれば非常にデバックしやすく、原因になっているところも分かると思います。
Javaの勉強頑張ってくださいね。

あと、もはや意味がないと予想されるリンクサイトに書かれていた、run関数は消してもらっても構いませんよ。

Comments

[swordone]

いや、runって少なくともこのプログラムにおいては肝だと思うんですが…

[stdio]

なんか肝心な部分が2つ書かれていてどちらが通っているのか実は判断できません。
肝かと言われれば、かつては肝だったですね。肝の部分がJava3に変わっている様に思ったので、消してもいいと言ったんですが...
あくまで予想で書いただけですので、肝だと主張するなら消しますが...

[swordone]

Java3?

いやだって、runを消したらこのプログラムは何もしなくなりますよ？

[stdio]

Java3は質問者が書いている上から3番目のcodeです。

> こちらを実行すると、無限ループになってしまいます。
って書いてますよね。多分魔改造しようとしてひとまずrunを変えたのでしょう。
貴方の主張通り、その場合Actorクラスが継承しているThreadももはや意味がなくなくなるのは確かです。

しかし質問の内容からして、きっと投稿されていないmain関数があるのだと予想できます。
しっかりとmainを書かず投稿してしまった質問自体に問題がありますが、質問文からそうではないかと予想して私はこのような発言をしました。説明が少なくて申し訳ないですがご了承いただきたい。
貴方はもう少し質問の内容を丁寧に読んだ方がいいと思いますよ。

[swordone]

ああそうか、Java3の書き方だと、それまでとは別のmainがないと機能しないか