Demilade Sonuga's blog

All posts

Refactoring VII

2023-05-05 · 12 min read

Finally, we have an event-handling scheme in place. We're almost done with the project. But before we get on with the rest, let's do some refactoring.

In this post, our refactoring is going to focus on code organization (sensibly grouping components), rather than readability (how easy it is to understand the code). To start, take a look at the blasterball/src directory:

assets/
    ...
allocator.rs
bitmap.rs
boxed_fn.rs
boxed.rs
event_hook.rs
font.rs
game.rs
gdt.rs
interrupts.rs
keyboard.rs
main.rs
pics.rs
port.rs
tss.rs
uefi.rs
vec.rs

Our project has grown considerably big. These files contain multiple components of the code that each serve specific purposes. There are many ways these files could be regrouped. We could separate out crates and then include them in blasterball/Cargo.toml as dependencies. We could group the files into separate modules that clearly define what they are used for and how they are related together. And so on. For this project, we're just going to group the files into separate modules. Why? Because I think more it's straightforward.

Do this yourself now.

The modules you came up with and the names you assigned to them will probably be different from mine because we don't think the exact same way. But, just for this project, we'll go with mine so that we'll be on the same page in future posts.

From the way I'm viewing this project directory, I think we can group modules like so:

  1. All Intel structures and interfaces into a module machine.
  2. Allocators and data structures involving heap allocation into alloc.
  3. Anything to do with displaying things on screen into display.
  4. main.rs, which drives execution, stays on its own.
  5. game.rs, too, stays on its own. There isn't anything to group it with (at least, in this scheme).
  6. event_hook.rs stays on its own since there isn't anything to group it with either.

After the refactoring and adding the mod.rs files to the new modules directories, the blasterball/src directory should look like this:

alloc/
    allocator.rs
    boxed_fn.rs
    boxed.rs
    mod.rs
    vec.rs
assets/
    ...
display/
    bitmap.rs
    font.rs
    mod.rs
machine/
    gdt.rs
    interrupts.rs
    keyboard.rs
    mod.rs
    pics.rs
    port.rs
    tss.rs
    uefi.rs
event_hook.rs
game.rs
main.rs

Remember to re-export the submodules in the new modules:

In alloc/mod.rs

pub mod allocator;
pub mod boxed_fn;
pub mod boxed;
pub mod vec;

In display/mod.rs

pub mod bitmap;
pub mod font;

In machine/mod.rs

pub mod gdt;
pub mod interrupts;
pub mod keyboard;
pub mod pics;
pub mod port;
pub mod tss;
pub mod uefi;

Updating the imports in main.rs

/* DELETED
pub mod port;
mod font;
use font::FONT;
mod boxed;
mod vec;
mod boxed_fn;
mod event_hook;
use event_hook::{EventInfo, EventKind};
mod allocator;
mod uefi;
use uefi::{SystemTable, Screen, PixelFormat, Pixel, pre_graphics_print_str, print_str, printint,
NO_OF_PIXELS_IN_A_ROW, BootServices, MemType};

mod bitmap;
use bitmap::{FileHeader, DIBHeader, ColorTable, Color, Bitmap, draw_bitmap, erase_bitmap};

// This needs to be imported so that the functions it defines may
// be used
use core::fmt::Write;

mod gdt;
use gdt::{Descriptor, GDT, SegmentRegister, SegmentSelector};

mod tss;
use tss::{TSS, load_tss};

mod interrupts;
use interrupts::{IDT, Entry, ServiceRoutine, ServiceRoutineWithErrCode, ServiceRoutineWithNoReturn};

mod game;

mod pics;
use pics::PICs;

mod keyboard;
use keyboard::Keyboard;
*/


mod machine;
mod alloc;
mod display;
mod event_hook;
mod game;

use core::fmt::Write;
use machine::{
port,
uefi::{SystemTable, BootServices, Screen, MemType, PixelFormat},
tss,
keyboard,
keyboard::Keyboard,
pics::PICs,
gdt,
gdt::{GDT, Descriptor, SegmentSelector, SegmentRegister},
tss::{TSS, load_tss},
interrupts,
interrupts::{IDT, ServiceRoutine, Entry, ServiceRoutineWithErrCode, ServiceRoutineWithNoReturn}
};
use alloc::{
allocator,
boxed_fn
};
use event_hook::{EventKind, EventInfo};

In game.rs

/* DELETED:
use crate::uefi::{Screen, NO_OF_PIXELS_IN_A_ROW, NO_OF_PIXELS_IN_A_COLUMN};
use crate::bitmap::{draw_bitmap, Bitmap};
*/

// NEW:
use crate::machine::uefi::{Screen, NO_OF_PIXELS_IN_A_ROW, NO_OF_PIXELS_IN_A_COLUMN};
use crate::display::bitmap::{draw_bitmap, Bitmap};

In event_hook.rs

use crate::keyboard::KeyEvent;
// DELETED: use crate::vec::Vec;
use crate::alloc::vec::Vec; // NEW
use crate::boxed_fn::BoxedFn;
use crate::allocator::{Allocator, get_allocator};
#[cfg(test)]
mod tests {
use super::*;
use crate::event_hook::EventInfo;
// DELETED: use crate::vec;
use crate::alloc::vec;

// ...Others
}

In uefi.rs

// DELETED: use crate::FONT;
use crate::display::font::FONT; // NEW
use core::fmt::Write;
use core::sync::atomic::{AtomicUsize, Ordering};
use core::ops::{Index, IndexMut};

In boxed_fn.rs

use crate::event_hook::EventInfo;
// DELETED: use crate::boxed::Box;
use crate::alloc::boxed::Box; // NEW
use crate::allocator::Allocator;

In bitmap.rs

/* DELETED:
use crate::{Screen, Pixel};
use crate::uefi::{NO_OF_PIXELS_IN_A_COLUMN, NO_OF_PIXELS_IN_A_ROW};
*/

use crate::machine::uefi::{
Screen,
Pixel,
NO_OF_PIXELS_IN_A_COLUMN,
NO_OF_PIXELS_IN_A_ROW
}; // NEW
use core::ops::Index;

And that's it for now. There is definitely still room for improvement. You can do that yourself. In the next post, we'll be focusing on the code itself and code quality in our refactoring.

Take Away

For the full code up to this point, go to the repo

In The Next Post

We'll continue refactoring