座標(x, y)が(0,0)-(COL-1, ROW-1)の外にハミ出したら行き止まりだよ、という単純なコードなのですが、
こういうのって、もっといい感じの書き方があるのではないでしょうか。
(修正:
(記事タイトルの表現が今一つ良くなかったので修正させていただきました。
解決済である点に変更はありません。)
JavaScript
1 if (x < 0) { 2 x = 0; 3 } else if (x > COL - 1) { 4 x = COL - 1; 5 } 6 if (y < 0) { 7 y = 0; 8 } else if (y > ROW - 1) { 9 y = ROW - 1; 10 }
(修正:
(改行が多くて10行にも及んでいますが、だからと言って見づらいわけでもないようです。
しかしながら基礎中の基礎の文法で書いているため、これで良いのかという疑問がありました。)
当面はJavaScriptの答えを頂きたいのですけど、他言語でも結構です。
ご教授よろしくお願いします。
気になる質問をクリップする
クリップした質問は、後からいつでもMYページで確認できます。
またクリップした質問に回答があった際、通知やメールを受け取ることができます。
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
回答4件
0
読みづらいとは
JavaScript
1if (x < 0) { 2 x = 0; 3} else if (x > COL - 1) { 4 x = COL - 1; 5} 6if (y < 0) { 7 y = 0; 8} else if (y > ROW - 1) { 9 y = ROW - 1; 10}
私はこのコードは「無駄な処理がない」良いコードと思います。
読みづらいと感じる具体的なポイントはどこでしょうか。
短いコードが良いとは限らない
「コードは短い方が良い」といわれますが、これは正確ではなく、同じ処理で短く書けるならの前提付きと認識しています。
例えば、次のようにコードを書き換えたとして、
JavaScript
1x = x > COL - 1 && COL - 1 || x > 0 && x || 0; 2y = y > ROW - 1 && ROW - 1 || y > 0 && y || 0;
コードの行数は減りましたが、読む苦労は増えていると感じます。
なぜなら、x,yの初期値分の条件式/代入値は元のコードにはない「無駄な処理」で、それが頭の中でアルゴリズムを描くまての手間を大きくしているからです。
良いコードとはアルゴリズムが短いコードであって、実際に書くコードが短い必要はありません。
(2018/08/10 13:17追記)
短縮化したコードの 1,0
が期待通りに動作しない為、コードを修正しました。
Re: himejiy3 さん
投稿2018/08/10 03:59
編集2018/08/12 00:21総合スコア18156
0
こんにちは。
ご質問に
もっといい感じの書き方があるのではないでしょうか。
とあります。何をもって いい感じ
のコードとするかは、回答者の私見に委ねられているものと思いますが、私は 「if
〜 else
を使わないで書いてみるとどうなりますか?」というお題と解釈したので、それに沿っての回答になります。
Math.min とMath.maxを使って、以下でどうでしょう?
javascript
1x = Math.min(COL-1, Math.max(0, x)); 2y = Math.min(ROW-1, Math.max(0, y)); 3
上記はあくまでコード量の少なさ、見た目のコードの短さにこだわった書き方です。
参考までに、stackoverflow にも同様の質問(javascriptではないですが。)
が挙がっていて、こちらの回答の中にも、Math.Min
と Math.Max
を使う、以下の回答があります。
ですが、上記の回答に対するこのコメントで指摘されているように、一回余分な比較が発生する余地があります。
余分な比較を発生させず、かつ一行で書くのであれば、上記の投稿で、質問者さんが最初に
I would like to clamp a value x to a range [a, b]:
x = (x < a) ? a : ((x > b) ? b : x);
と書いているように、三項演算子の利用も検討されるとよいかと思います。
追記1
ご質問の冒頭に、
座標(x, y)が・・・
とありましたので、座標上の点を処理するコードのリファクタという観点で、以下を追記します。やることは以下です。
- 座標上の点クラス
Point
を作成 Point
を拡張して、あらかじめ設定された最小値と最大値の範囲に収まるように各座標が調整されるRestrictedPoint
を作成- なお以下では、応用であることを明確にするため、
Point
インスタンスは、xyz軸を持つ3次元空間の点としています。
上記にそって、作ったコードが以下です。
javascript
1class Point { 2 constructor(x, y, z) { 3 this.x = x; 4 this.y = y; 5 this.z = z; 6 } 7} 8 9class RestrictedPoint extends Point { 10 constructor(x, y, z) { 11 super(x, y, z); 12 this.adjust(); 13 } 14 15 adjust() { 16 for ( const [axis, value] of Object.entries(this)) { 17 const config = this.constructor.config[axis]; 18 this[axis] = Math.min(config.max, Math.max(config.min, value)); 19 } 20 } 21} 22 23RestrictedPoint.config = { 24 x: { min: 0, max: 99 }, // X座標は0以上99以下 25 y: { min: 0, max: 199 }, // Y座標は0以上199以下 26 z: { min: 100, max: 399 } // Z座標は100以上399以下 27}; 28 29const p1 = new Point(-1, 230, 50); 30console.log(p1); // => Object {x=-1, y=230, z=50} 31 32const p2 = new RestrictedPoint(-1, 230, 50); 33console.log(p2); // => Object {x=0, y=199, z=100} 34
以下、上記を試行するサンプルです。
https://jsfiddle.net/jun68ykt/cs86vtj3/15/
追記2
もうひとつ蛇足ですが、上記のRestrictedPoint
の書き方として、アクセサを使う手もありそうです。
javascript
1class Point { 2 constructor(x=0, y=0, z=0) { 3 this._x = x; 4 this._y = y; 5 this._z = z; 6 } 7 8 get x() { return this._x; } 9 set x(value) { this._x = value; } 10 11 get y() { return this._y; } 12 set y(value) { this._y = value; } 13 14 get z() { return this._z; } 15 set z(value) { this._z = value; } 16 17 toString() { 18 return `(x: ${this.x}, y: ${this.y}, z: ${this.z})`; 19 } 20} 21 22class RestrictedPoint extends Point { 23 24 static config() { 25 return { 26 x: { min: 0, max: 99 }, // X座標は0以上99以下 27 y: { min: 0, max: 199 }, // Y座標は0以上199以下 28 z: { min: 100, max: 399 } // Z座標は100以上399以下 29 }; 30 } 31 32 get(axis) { 33 const conf = this.constructor.config()[axis]; 34 return Math.min(conf.max, Math.max(conf.min, this[`_${axis}`])); 35 } 36 37 get x() { return this.get('x'); } 38 set x(value) { this._x = value; } 39 40 get y() { return this.get('y'); } 41 set y(value) { this._y = value; } 42 43 get z() { return this.get('z'); } 44 set z(value) { this._z = value; } 45 46} 47 48const p1 = new Point(-1, 230, 50); 49console.log(`${p1}`); // => (x: -1, y: 230, z: 50) 50 51p1.x = 1000; 52console.log(p1.x); // => 1000 53 54const p2 = new RestrictedPoint(-1, 230, 50); 55console.log(`${p2}`); // => (x: 0, y: 199, z: 100) 56 57p2.x = 1000; 58console.log(p2.x); // => 99 59
以下に上記のコードを上げました。
https://jsfiddle.net/jun68ykt/cs86vtj3/73/
アクセサを使う利点としては、上記のコードの最後の2行に書いてあるように、p2.x = 1000;
と代入したつもりでも、 p2.x
は X座標の上限値の99
を返すようにできることです。
投稿2018/08/09 19:11
編集2018/08/11 00:42総合スコア9058
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2018/08/10 05:02 編集
2018/08/10 05:11
2018/08/10 06:20
0
こういう書き方もできますよ。
js
1x = Math.min( 2 Math.max(x, 0), 3 COL - 1 4) 5y = Math.min( 6 Math.max(y, 0), 7 ROW - 1 8)
追記
他の回答でも言われているように関数化することでコードの見通しが良くなります。
js
1const clamp = (n, min, max) => 2 Math.min( 3 Math.max(n, min), 4 max 5 ) 6x = clamp(x, 0, COL - 1) 7y = clamp(y, 0, ROW - 1)
投稿2018/08/09 19:10
編集2018/08/10 00:30退会済みユーザー
総合スコア0
0
ベストアンサー
おとなしく関数化しましょう。
x = LimitRange(x, 0, COL-1); y = LimitRange(y, 0, ROW-1); function LimitRange(value, min, max) { if (value < min) { return min; } if (value > max) { return max; } return value; }
投稿2018/08/10 00:10
編集2018/08/10 02:49総合スコア4524
あなたの回答
tips
太字
斜体
打ち消し線
見出し
引用テキストの挿入
コードの挿入
リンクの挿入
リストの挿入
番号リストの挿入
表の挿入
水平線の挿入
プレビュー
質問の解決につながる回答をしましょう。 サンプルコードなど、より具体的な説明があると質問者の理解の助けになります。 また、読む側のことを考えた、分かりやすい文章を心がけましょう。
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2018/08/10 04:11
2018/08/10 04:21
2018/08/10 04:44
2018/08/10 05:18
2018/08/12 00:28