Getting rid of unnecessary explicit conversions with named constants


#1

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.

See also


#2

You could move the multiples outside of the cast - e.g. (maze!(self).height() * TWH) as i32.


#3

maze!(self).height() is usize while TWH is u32, so no.

But I’m not asking for code review, the code was just an example to make a point.


#4

This is a problem I face with winapi. I hope this gets resolved somehow.


#5

I’d much rather have an explicit “any number” type that can be converted implicitly to numeric types if it can be done losslessly and at compile-time.

a way to represent such a type could be const X = 42; or by simply adding the “any number” type as some kind of lang-item.


#6

Mad fever dream: special types for the various literals that must be coerced into a concrete type during compilation. So Rust could have an IntLit type which is a real type (probably a bignum), which could be used with CTFE and constant folding, put in consts, etc., but which has to coerce to some “runtime” type before or during codegen.