This isn't regarding the code per-se, but I would recommend uploading the project to GitHub and breaking each module out into its own file. It makes the project a lot easier to get an understanding of at a glance, and also means people can check the code out and run it on their own computers.
Instead of a Cat
having CatColour
and CatRace
fields, I'd just call them Colour
and Race
. There's no real need for the extra Cat
prefix.
Instead of the explicit Cat::to_string()
method, you should look at implementing std::fmt::Display
. That'll allow you to println!()
a Cat
directly, and the ToString
trait (which has a to_string()
method) gets automatically implemented for anything implementing Display
.
Instead of having a CatColour
enum with a pre-defined set of colours, what about allowing a colour to be any RGB value and define constants like CatColour::WHITE
for the common colours? One day someone is going to want to add a tabby cat or some random colour like turqouise, so you may want to make this sort of thing more flexible...
It depends on how far down the rabbit-hole you want to go, of course.
You could say the same about CatRace
. An enum
is best used when you've got a closed set of possibilities known ahead of time, but the CatRace
seems like it'll have a possibly unbounded number of values. Especially when you take into account things like mixed races. I'm guessing the easiest solution would be to have a CatRace
struct with a single user-defined name: String
field.
If you're using cargo
I'd recommend checking out some sort of readline library to read input, that way your users get nice things like tab-completion and history and it feels like a proper REPL. I've had good experiences with rustyline
.
You don't need to use io::Write::flush(&mut io::stdout())
, you should be able to do io::stdout().flush()
if the std::io::Write
trait is in scope.
When reading input, you probably don't want to print eprintln!("Corrupted input '{}' given!", input)
and exit. A lot of tools will read input in a loop, printing out an error message and prompting again when there is an error. Your get()
function may want to accept a prompt: &str
parameter which gets print!()
-ed before trying to read a line of input.
Looking at your DataManager
trait I'm guessing you come from a more stereotypical OO language like C# or Java? The naming and methods look like some of the code I've seen people use when writing Android apps (e.g. "Data Access Object"). That's not necessarily a bad thing though, using a trait gives you a seam for testing and swapping out backends (e.g. mysql vs postgres vs file vs firebase). You should probably rename it to something which indicates it's in charge of storing and retrieving Cat
s... Maybe Cats
or CatDatabase
or CatStore
? I dunno, naming things is hard 
Something to keep in mind with your DataManager
is that each operation can fail (due to a unique SQL constraint, broken database connection, etc.) so each method should return some sort of Result
.
I don't think DataManager::read()
should return a &Vec<Cat>
. Because you're returning a reference it means the Cat
s are actually stored inside the DataManager
somewhere (e.g. in a buffer/cache). Additionally, because of lifetimes it means you can't use the DataManager
again until the returned value gets dropped. I'd just return a Result<Vec<Cat>, SomeDatabaseError>
, returning a reference won't make a difference in performance if you're already talking to a database.
The DataManager::get()
and DataManager::delete()
methods should probably accept &str
instead of &String
. A String
is actually a pointer to a str
so you've got a bit of double-indirection going on. Plus if I wanted to delete all cats with the hard-coded name "steve"
I'd need to copy it to an intermediate String
, which is kinda annoying.
This review is getting a bit long and would probably be easier to do using a proper review interface like that provided by GitLab
or GitHub
, but hopefully you find some of my comments useful.
Despite the large number of points above I'm actually quite impressed with your code so far! While a couple things aren't the most idiomatic, the code overall is well structured and quite readable, it feels like you're the kind of coder people would want to work with.
I'm keen to see how far this goes 