4
\$\begingroup\$

Rust is one of my first forays on a strongly typed language, beginning to like it's strictness, and am looking to improve my comprehension.

Rules

Fizzbuzz is a programming challenge with the following rules for output;

  • n mod 3 => "Fizz"

  • n mod 5 => "Buzz"

  • n mod 3 && n mod 5 => "Fizzbuzz"

  • else n => n

Examples;

  • 10 => "Buzz"
  • 11 => "11"
  • 12 => "Fizz"
  • 13 => "13"
  • 14 => "14"
  • 15 => "Fizzbuzz"

Questions

  • Are there any mistakes that the compiler hasn't pestered my about?

  • The iterator increments or decrements, any other directions it should go?

  • It's not the shortest implementation, not my primary goal, but do any bits of code smell?

  • What other features would be good to add for the learning experience?


Steps to Reproduce

Initialization

cargo init fizzbuzz

cd fizzbuzz

Add argparse as dependency to Cargo.toml file

[dependencies]
argparse = "0.2.2"

See Source Code for contents of src/main.rs file.

Compiling and running

cargo build

./target/debug/fizzbuzz --begin 1 --end 26
./target/debug/fizzbuzz --begin 100 --end 74

Source Code

src/main.rs

extern crate argparse;
use argparse::{ArgumentParser, Store, StoreTrue};


/// **Licensing**
///
/// Rust Fizzbuzz
/// Copyright (C) 2020  S0AndS0
///
/// This program is free software: you can redistribute it and/or modify
/// it under the terms of the GNU Affero General Public License as published
/// by the Free Software Foundation; version 3 of the License.
///
/// This program is distributed in the hope that it will be useful,
/// but WITHOUT ANY WARRANTY; without even the implied warranty of
/// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
/// GNU Affero General Public License for more details.
///
/// You should have received a copy of the GNU Affero General Public License
/// along with this program.  If not, see <https://www.gnu.org/licenses/>.


/// Data structure for mutable states within `Game`
#[derive(Clone)]
struct State {
    fizz: bool,
    buzz: bool,
    count: u32,
    message: String
}


impl State {
    /// Method for defaulting values other than `count`
    fn new(count: Option<u32>) -> Self {
        let count: u32 = match count {
            Some(i) => { i },
            None => { 1 }
        };

        return State {
            fizz: false,
            buzz: false,
            count: count,
            message: String::from(""),
        }
    }
}


/// Data structure of what `Game` is concerned with
struct Game {
    state: State,
    limit: u32,
}


impl Game {
    /// Method for initializing game with counter and limit
    fn new(count: Option<u32>, limit: Option<u32>) -> Self {
        let limit: u32 = match limit {
            Some(i) => { i },
            None => { 16 }
        };

        Game { state: State::new(count), limit: limit }
    }
}


impl Iterator for Game {
    /// Return type of iterator
    type Item = State;

    /// Called implicitly by `for` loops
    fn next(&mut self) -> Option<State> {
        self.state.fizz = false;
        self.state.buzz = false;
        self.state.message = String::from("");

        if self.state.count % 3 == 0 {
            self.state.fizz = true;
            self.state.message.push_str("fizz");
        }

        if self.state.count % 5 == 0 {
            self.state.buzz = true;
            self.state.message.push_str("buzz");
        }

        if self.state.message.is_empty() {
            self.state.message = self.state.count.to_string();
        } else if let Some(r) = self.state.message.get_mut(0..1) {
            r.make_ascii_uppercase();
        }

        if self.state.count != self.limit {
            if self.state.count < self.limit {
                self.state.count += 1;
            } else {
                self.state.count -= 1;
            }

            Some(self.state.clone())
        } else {
            None
        }
    }
}


/// Called automatically by `cargo run` command or executing compiled binary
fn main() {
    let mut begin: u32 = 1;
    let mut end: u32 = 100;
    let mut verbose = false;

    // Limit borrowing scope for `ap.refer` methods.
    {
        let mut ap = ArgumentParser::new();
        ap.set_description("Iterator based Fizzbuzz example writen in Rust");

        ap.refer(&mut begin).add_option(
            &["--begin", "--start"],
            Store,
            "Where iteration begins"
        );

        ap.refer(&mut end).add_option(
            &["--end", "--stop"],
            Store,
            "Where iteration ends"
        );

        ap.refer(&mut verbose).add_option(
            &["-v", "--verbose"],
            StoreTrue,
            "How _noisy_ to be"
        );

        ap.parse_args_or_exit();
    }

    if verbose {
        println!("begin -> {}", begin);
        println!("end -> {}", end);
    }

    let game: Game = Game::new(Some(begin), Some(end));
    for state in game {
        println!("{}", state.message);
    }
}

Attribution

\$\endgroup\$
  • \$\begingroup\$ Excellent suggestion! I've added a bit on how Fizzbuzz is usually described... though I seem to remember it being a drinking game too, so rules of output could differ. \$\endgroup\$ – S0AndS0 Jan 25 at 2:25
3
\$\begingroup\$

extern crate argparse; 2018 Rust edition doesn't require this, see.


return State {
    fizz: false,
    buzz: false,
    count: count,
    message: String::from(""),
}

Here return is not idiomatic and you can omit count:, more info here and here.

Same thing Game { state: State::new(count), limit: limit }. Also, this time you did omit the return keyword, choice your style but you should be consistent.


self.state.fizz = false;

if self.state.count % 3 == 0 {
    self.state.fizz = true;
    self.state.message.push_str("fizz");
}

This is also odd prefer use else branch:

self.state.fizz = if self.state.count % 3 == 0 {
    self.state.message.push_str("fizz");
    true
}
else {
    false
}; 

self.state.message = String::from(""); maybe prefer use clean() to reuse previous allocate memory.


if self.state.message.is_empty() {
    self.state.message = self.state.count.to_string();
} else if let Some(r) = self.state.message.get_mut(0..1) {
    r.make_ascii_uppercase();
}

As you already linked it, but I think the best is to avoid situation where you need to capitalized first letter. That could be done using the fact that n % 3 == 0 and n % 5 == 0 is only true for n % 15 so you could add a if condition in your algorithm to remove this issue.


if self.state.count != self.limit {
    if self.state.count < self.limit {
        self.state.count += 1;
    } else {
        self.state.count -= 1;
    }

You just reinvent Range feature, there is a much better way to do this in rust, see end example.


Some(self.state.clone()), you could return a &str, see this answer for how to do it.


I have not much to say about argparse.


let mut begin: u32 = 1;, let mut end: u32 = 100;, let game: Game = Game::new(Some(begin), Some(end));, this is not useful let rust infer your variable type in general.


Also, a general critic about your algorithm, your State is odd, there is no point to keep it in your Game iterator. The only thing you need to keep is count, and State neither use count fizz or buzz, you only use the string. So State is basically just a String and contains count that should belong to Game and two useless variable that you don't use. See my end example to see a better way to manage it.


The following is an example of what you could have do, removing your strange algorithm but trying to keep your way.

use argparse::{ArgumentParser, Store, StoreTrue};

#[derive(Debug)]
enum State {
    Fizz,
    Buzz,
    FizzBuzz,
    None(u32),
}

use std::fmt;

impl fmt::Display for State {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match self {
            State::Fizz => write!(f, "Fizz"),
            State::Buzz => write!(f, "Buzz"),
            State::FizzBuzz => write!(f, "FizzBuzz"),
            State::None(n) => write!(f, "{}", n),
        }
    }
}

struct Game<I> {
    inner: I,
}

use std::iter::IntoIterator;

impl<I> Game<I>
where
    I: Iterator,
{
    fn new<U>(i: U) -> Self
    where
        U: IntoIterator<IntoIter = I, Item = I::Item>,
    {
        Game {
            inner: i.into_iter(),
        }
    }
}

impl<I> Iterator for Game<I>
where
    I: Iterator<Item = u32>,
{
    type Item = State;

    fn next(&mut self) -> Option<State> {
        let n = self.inner.next()?;

        let state = if n % 15 == 0 {
            State::FizzBuzz
        } else if n % 3 == 0 {
            State::Fizz
        } else if n % 5 == 0 {
            State::Buzz
        } else {
            State::None(n)
        };

        Some(state)
    }
}

fn main() {
    let mut begin = 1;
    let mut end = 100;
    let mut verbose = false;

    {
        let mut ap = ArgumentParser::new();
        ap.set_description("Iterator based Fizzbuzz example writen in Rust");

        ap.refer(&mut begin)
            .add_option(&["--begin", "--start"], Store, "Where iteration begins");

        ap.refer(&mut end)
            .add_option(&["--end", "--stop"], Store, "Where iteration ends");

        ap.refer(&mut verbose)
            .add_option(&["-v", "--verbose"], StoreTrue, "How _noisy_ to be");

        ap.parse_args_or_exit();
    }

    if verbose {
        println!("begin -> {}", begin);
        println!("end -> {}", end);
    }

    let game = Game::new(begin..end);
    for state in game {
        println!("{}", state);
    }
}
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Thank you! Your way seems much cleaner than what I was writing, that range iterator trick is supper snazzy, and your use of enum is fantastic... I'll certainly have to spend more time with your code, and I'll be sure to mark this as the accepted answer if there are no other contenders by the end of the day. \$\endgroup\$ – S0AndS0 Jan 24 at 17:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.