java - ステートメントハックの場合、これを書くより良い方法はありますか?

Question

私は単純な横スクロールゲームを書くための実験的な最初のショットを取っています。プレーヤーがある方向に移動しすぎた場合に画面を再中央揃えするためのこのコードがあります。

もっと良い書き方はないでしょうか？言語を悪用しているように感じますが、うまくいったので大丈夫かもしれません。

public void adjustFrameIfNecessary()
{
    int dx, dy;
    if ((dx = (GAME_WIDTH - GAME_WIDTH / 3) - player.x) < 0 || (dx = GAME_WIDTH / 3 - player.x) > 0 || (dx = 0) == 0);
    if ((dy = (GAME_HEIGHT - GAME_HEIGHT / 3) - player.y) < 0 || (dy = GAME_HEIGHT / 3 - player.y) > 0 || (dy = 0) == 0);

    if(dx != 0 || dy != 0)
    {
        for (Drawable shiftMe : drawables)
        {
            shiftMe.unconditionalShift(dx, dy);
        }
    }

}

編集

皆様のご意見は、より読みやすくするために、このように変更しました

public void adjustFrameIfNecessary()
{
    int dx, dy;

    assignX:
    {
        dx = (GAME_WIDTH - GAME_WIDTH / 3) - player.x;
        if(dx < 0) break assignX;
        dx = GAME_WIDTH / 3 - player.x;
        if(dx > 0) break assignX;
        dx = 0;
    }

    assignY:
    {
        dy = (GAME_HEIGHT - GAME_HEIGHT / 3) - player.y;
        if(dy < 0) break assignY;
        dy = GAME_HEIGHT / 3 - player.y;
        if(dy > 0) break assignY;
        dy = 0;
    }

    if (dx != 0 || dy != 0)
    {
        for (Drawable shiftMe : drawables)
        {
            shiftMe.unconditionalShift(dx, dy);
        }
    }

}

これは良いですか？

編集2

public void adjustFrameIfNecessary()
{
    int dx = calculateShift(GAME_WIDTH, frameReference.x);
    int dy = calculateShift(GAME_HEIGHT, frameReference.y);

    if (dx != 0 || dy != 0)
    {
        for (Drawable shiftMe : drawables)
        {
            shiftMe.unconditionalShift(dx, dy);
        }
    }
}

それはもう明らかだと思います。みんな、ありがとう。

score 5 · Accepted Answer

ifなぜあなたがそれらの声明を一番上に持っているのか、私にはまったくわかりません。変数を初期化するときは、単に変数を割り当てる必要があります。

dx = (GAME_WIDTH - GAME_WIDTH / 3) - player.x)
dy = (GAME_HEIGHT - GAME_HEIGHT / 3) - player.y)

ifあなたの声明に関係なく、彼らはそれに割り当てられます。3 番目の条件を再取得した場合は、すでにゼロであるため、割り当てる理由はありません。2 つのステートメントを破棄し、if代わりに単純な変数の割り当てに固執します。条件をループより上に保ちforます。これが唯一の確認事項です。

score 4 · Accepted Answer

あなたのifステートメントは読みにくいです。あなた (または他の誰か) が後で変更する必要がある場合、それは非常に難しく、エラーが発生しやすい可能性があります。

機能するだけでなく、保守しやすいコードを作成することは良い習慣です。機能するようになったので、より保守しやすくする必要があります。

if ステートメントを意味のある変数に分割し、複数の行を使用します。あなたはゴルフコードコンテストのために書いているのではありませんか?

後で感謝します。

score 1 · Accepted Answer

これはどう？

/* calculate offset to center */
dx = X_current_center - player.x;
dy = Y_current_center - player.y;

if (abs(dx) > MARGIN || abs(dy) > MARGIN)
    /* recenter */

score 1 · Accepted Answer

読めませんが、あなたの言いたいことはわかります。ただし、x と y の両方に同じチェックを再利用しないのはなぜでしょうか? また、三項演算子を使用すると、ハックが 1 つ少なくなります。

static int toBounds(int max, int curr) {
  int ret;
  return ((ret = (max - max/3) - curr) < 0 || (ret = max/3 - curr) > 0)? ret : 0;
}

score 1 · Accepted Answer

そのようなコードを改善するアイデアは、通常、数学を使用することです:)

初めまして、長文失礼しました。もしあなたが私に直接会ったことがあるなら、この話題について私に聞かないでください、私は話すのをやめません.

短縮版

これを行う：

/**
 * Computes the distance from a point x to an interval [a,b]
 */
public static int distanceToInterval( int x, int a, int b ){
   return x<a? (a-x): ( x>b? (b-x):0 ); 
   // alternative: 
   // return Math.max( a - x, 0 ) + Math.min( b - x, 0 ); 
}

// then use as 
int dx = distanceToInterval( player.x, GAME_WIDTH/3, GAME_WIDTH*2/3 ); 
int dy = distanceToInterval( player.y, GAME_HEIGHT/3, GAME_HEIGHT*2/3 );

ロングバージョン

あなたの最初の声明

if ((dx = (GAME_WIDTH - GAME_WIDTH / 3) - player.x) < 0 || 
    (dx = GAME_WIDTH / 3 - player.x) > 0 || 
    (dx = 0) == 0);

これは、...にすることで少しきれいにすることができます。

...長いバージョン

dx1 = GAME_WIDTH*2/3 - player.x; 
dx2 = GAME_WIDTH*1/3 - player.x; 
dx3 = 0; 

if( dx1 < 0 ) dx = dx1; 
else if( dx2 > 0 ) dx = dx2; 
else dx = dx3;

これは私見です。私はあなたが何をしようとしているのかを見て、それをきちんとした文章で表現することができます: プレーヤーが画面の中央 3 分の 1 の範囲内にない場合、画面を移動したいと思います

さらに明確にする

if( player.x < GAME_WIDTH/3 ) dx = GAME_WIDTH/3 - player.x;  // positive num
else if( player.x > GAME_WIDTH*2/3 ) dx = GAME_WIDTH*2/3 - player.x; // negative num
else dx = 0; // no shift

ステートメントを並べ替えたことがわかります。最初に左の境界をチェックし、次に右の境界をチェックします。あなたが右から左に読む国に来たら、他の方向がより直感的にわかるかもしれません:)

これは私が選ぶ解決策です。短く、読みやすく、完璧です:)

ただし、より簡潔にしたい場合は、さらに一歩進むことができます

数学のトリックをする

3 つのケースは完全に排他的であることを理解する必要があります。同時に発生することはありません。より多くの変数の使用に戻りましょう:

// on the left side we shift a positive number, or not at all 
dx1 = Math.max( GAME_WIDTH/3 - player.x, 0 ); 
// on the right side we shift a negative amount, or not at all 
dx2 = Math.min( GAME_WIDTH*2/3 - player.x, 0 );

これの美しさを見てください：

ならGAME_WIDTH/3-player.x > 0、GAME_WIDTH*2/3 - player.x > 0 こうしてMath.min(...,0) = 0
ならGAME_WIDTH*2/3 - player.x < 0、GAME_WIDTH*1/3 - player.x < 0 こうしてMath.max(...,0) = 0

これは、単純に 2 つを加算して合計シフトを計算できることを意味します。コードは次のようになります。

int dx = 
     Math.max( GAME_WIDTH/3 - player.x, 0 ) + // too far to the left? 
     Math.min( GAME_WIDTH*2/3 - player.x, 0 ); // or too far to the right?

もう少し抽象化

選択したバリアントが何であれ、これをメソッドに入れるのは非常に簡単になりました。しかし、この特殊なケースよりももう少し意味のあるものにしましょう。私は提案します

/**
 * Computes the distance from a point x to an interval [a,b]
 */
public static int distanceToInterval( int x, int a, int b ){
   return 
       Math.max( a - x, 0 ) + // too far to the left? 
       Math.min( b - x, 0 ); // or too far to the right? 
}

// now use as 
int dx = distanceToInterval( player.x, GAME_WIDTH/3, GAME_WIDTH*2/3 ); 
int dy = distanceToInterval( player.y, GAME_HEIGHT/3, GAME_HEIGHT*2/3 );

psこの投稿全体を通して、私がに置き換えGAME_WIDTH - GAME_WIDTH/3たことに注意してください。GAME_WIDTH*2/3これら2つは整数演算でわずかに異なる結果をもたらしますが、私はより直感的であるため、短いバージョンを好みます（「画面の3分の2」対「画面全体と3分の1戻る」）。

java - ステートメントハックの場合、これを書くより良い方法はありますか?

5 に答える 5

短縮版

ロングバージョン

Related

Reference