Recently, I replaced a magic number with a named constant in my project. Here is the diff:
diff --git a/src/main.rs b/src/main.rs
index e6076a9..18d1833 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -40,6 +40,9 @@ use maze::Maze;
use player::Player;
+/// Tile width/height
+const TWH: u32 = 32;
+
struct Game {
mazes: Vec<Maze>,
level: usize,
@@ -177,9 +180,9 @@ impl Game {
}
cam_x = ::util::clamp(self.player.pix_pos().x - 320,
- 0, (maze!(self).width() as i32 * 32) - 640);
+ 0, (maze!(self).width() as i32 * TWH as i32) - 640);
cam_y = ::util::clamp(self.player.pix_pos().y - 240,
- 0, (maze!(self).height() as i32 * 32) - 480);
+ 0, (maze!(self).height() as i32 * TWH as i32) - 480);
self.renderer.clear();
maze!(self).draw(&self.tiles, &mut self.renderer, cam_x, cam_y);
diff --git a/src/maze/mod.rs b/src/maze/mod.rs
index e218bdd..ad1e793 100644
--- a/src/maze/mod.rs
+++ b/src/maze/mod.rs
@@ -1,6 +1,8 @@
mod gen;
mod tiles;
+use TWH;
+
use util::{
Grid2d,
Pos2d,
@@ -44,28 +46,28 @@ impl Maze {
}
}
pub fn draw(&self, tileset: &Texture, renderer: &mut Renderer, x_from: i32, y_from: i32) {
- let tx_from = ::std::cmp::max(x_from / 32, 0) as usize;
- let ty_from = ::std::cmp::max(y_from / 32, 0) as usize;
- let tx_to = ::std::cmp::min(tx_from + (640 / 32) + 1, self.grid.width());
- let ty_to = ::std::cmp::min(ty_from + (480 / 32) + 1, self.grid.height());
+ let tx_from = ::std::cmp::max(x_from / TWH as i32, 0) as usize;
+ let ty_from = ::std::cmp::max(y_from / TWH as i32, 0) as usize;
+ let tx_to = ::std::cmp::min(tx_from + (640 / TWH as usize) + 1, self.grid.width());
+ let ty_to = ::std::cmp::min(ty_from + (480 / TWH as usize) + 1, self.grid.height());
for y in ty_from..ty_to {
for x in tx_from..tx_to {
- let xx = (x as i32 * 32) - x_from;
- let yy = (y as i32 * 32) - y_from;
- let target_rect = Rect::new(xx, yy, 32, 32).unwrap();
+ let xx = (x as i32 * TWH as i32) - x_from;
+ let yy = (y as i32 * TWH as i32) - y_from;
+ let target_rect = Rect::new(xx, yy, TWH, TWH).unwrap();
let tile = self.grid.get(x, y);
- let source_rect = Rect::new(tile.image_pos.x * 32,
- tile.image_pos.y * 32,
- 32, 32).unwrap();
+ let source_rect = Rect::new(tile.image_pos.x * TWH as i32,
+ tile.image_pos.y * TWH as i32,
+ TWH, TWH).unwrap();
renderer.copy(tileset, source_rect, target_rect);
}
}
- let target_rect = Rect::new((self.stairs_up.x as i32 * 32) - x_from,
- (self.stairs_up.y as i32 * 32) - y_from, 32, 32).unwrap();
- renderer.copy(tileset, Rect::new(64, 0, 32, 32).unwrap(), target_rect);
- let target_rect = Rect::new((self.stairs_down.x as i32 * 32) - x_from,
- (self.stairs_down.y as i32 * 32) - y_from, 32, 32).unwrap();
- renderer.copy(tileset, Rect::new(96, 0, 32, 32).unwrap(), target_rect);
+ let target_rect = Rect::new((self.stairs_up.x as i32 * TWH as i32) - x_from,
+ (self.stairs_up.y as i32 * TWH as i32) - y_from, TWH, TWH).unwrap();
+ renderer.copy(tileset, Rect::new(2 * TWH as i32, 0, TWH, TWH).unwrap(), target_rect);
+ let target_rect = Rect::new((self.stairs_down.x as i32 * TWH as i32) - x_from,
+ (self.stairs_down.y as i32 * TWH as i32) - y_from, TWH, TWH).unwrap();
+ renderer.copy(tileset, Rect::new(3 * TWH as i32, 0, TWH, TWH).unwrap(), target_rect);
}
pub fn stairs_up_pos(&self) -> Pos {
self.stairs_up
diff --git a/src/player.rs b/src/player.rs
index ee05dd2..03a85a4 100644
--- a/src/player.rs
+++ b/src/player.rs
@@ -1,5 +1,6 @@
use maze::Pos;
use util::Pos2d;
+use TWH;
use sdl2::render::{
Renderer,
@@ -21,13 +22,13 @@ impl Player {
pub fn new(starting_pos: Pos) -> Self {
Player {
tile_pos: starting_pos,
- pix_pos: Pos2d::new(starting_pos.x as i32 * 32, starting_pos.y as i32 * 32),
+ pix_pos: Pos2d::new(starting_pos.x as i32 * TWH as i32, starting_pos.y as i32 * TWH as i32),
}
}
pub fn draw(&self, tiles: &Texture, renderer: &mut Renderer, x_from: i32, y_from: i32) {
- let src_rect = Rect::new(128, 0, 32, 32).unwrap();
+ let src_rect = Rect::new(128, 0, TWH, TWH).unwrap();
let dst_rect = Rect::new(self.pix_pos.x - x_from,
- self.pix_pos.y - y_from, 32, 32).unwrap();
+ self.pix_pos.y - y_from, TWH, TWH).unwrap();
renderer.copy(tiles, src_rect, dst_rect);
}
pub fn move_left(&mut self) {
@@ -47,15 +48,15 @@ impl Player {
}
pub fn set_tile_pos(&mut self, pos: Pos) {
self.tile_pos = pos;
- self.pix_pos = pos * 32;
+ self.pix_pos = pos * TWH as usize;
}
pub fn pix_pos(&self) -> Pos2d<i32> {
self.pix_pos
}
pub fn update_step(&mut self) -> StepUpdate {
let speed = 2;
- let new_x = self.tile_pos.x as i32 * 32;
- let new_y = self.tile_pos.y as i32 * 32;
+ let new_x = self.tile_pos.x as i32 * TWH as i32;
+ let new_y = self.tile_pos.y as i32 * TWH as i32;
if self.pix_pos.x > new_x {
self.pix_pos.x -= speed;
} else if self.pix_pos.x < new_x {
As you can see, it significantly inflated the source code with a lot of unnecessary foo as T conversions, which were not required when just presenting the unnamed literal.
One alternative to this would be using a macro for defining the constant, which would yield an untyped literal, but macros don't follow normal scoping rules, and twh!() is less clear in clarifying we are dealing with a named constant than TWH.
I want to start a discussion of how to get rid of these unnecessary explicit conversions, and possibly write up an RFC if we can come up with something reasonable.
Proposal 1: Allowing implicit lossless conversions
Since we are dealing with constant values here, the compiler can know whether the value can be represented in the target type without information loss. In cases where conversion can be made without loss, the compiler could do the conversion implicitly, without requiring an explicit conversion from the user.