response.rs

type FencerID = usize;// Make this explicit: struct FencerId(usize); and use #[derive(Clone, Copy, Hash, PartialEq, Eq)]

#[derive(Debug, Hash, Eq, PartialEq)]
struct FencerData {
    name: String,
    // rating: some kind of enum?
}

impl FencerData {
    fn new(name: &str) -> FencerData {// MG: Also it's fine to take a String here. Prevents extra clones if I do eg Fencer::new(format!("..."))
        FencerData { name: name.into() } // MG: Prefer into_owned to make this more explicit
    }
}

#[derive(Clone)]
struct PoolScore {
    touches: u32,
    victory: bool,
}

/* MG: Dont need this. Use the state of a hashmap's .get()
#[derive(Clone)]
enum PoolEntry {
    Complete(PoolScore),
    NotYet,
    SameFencer,
}*/

// MG: I don't think u need this if u apply other suggested changes
impl PoolEntry {
    fn for_scoring(&self) -> PoolScore {
        match self {
            PoolEntry::Complete(score) => score.clone(),
            _ => PoolScore {
                touches: 0,
                victory: false,
            },
        }
    }
}

struct Pool {
    /// The fencers in this pool
    fencers: Vec<FencerID>,
    /// A mapping of fencers to their scores. (F1, F2) -> F1's score.
    scores: Vec<Vec<PoolEntry>>, // MG: Prefer a HashMap<(FencerId, FencerId), PoolEntry>
}

impl Pool {
    fn new(fencers: Vec<FencerID>) -> Pool {
        // MG: This can be very simple if u use a hashmap. No init needed.
        let size = fencers.len();
        let mut pool = Pool {
            fencers,
            scores: vec![vec![PoolEntry::NotYet; size]; size],
        };
        for i in 0..size {
            pool.scores[i][i] = PoolEntry::SameFencer;
        }
        pool
    }

    fn size(&self) -> usize {
        // MG: name this like.. num_fencers or smth maybe?
        self.fencers.len()
    }

    /*MG: Don't need these... Just use HashMap's own .get_mut

    fn get_cell(&self, i: usize, j: usize) -> Option<&PoolEntry> {
        self.scores.get(i).and_then(|row| row.get(j))
    }

    fn get_cell_mut(&mut self, i: usize, j: usize) -> Option<&mut PoolEntry> {
        self.scores.get_mut(i).and_then(|row| row.get_mut(j))
    }

    */

    fn get_bout(
        &mut self,
        fencer_left: usize,
        fencer_right: usize,
    ) -> Option<(PoolEntry, PoolEntry)> {
        // Get the scores
        let score_left = self.get_cell(fencer_left, fencer_right);
        let score_right = self.get_cell(fencer_right, fencer_left);

        match (score_left, score_right) {
            (Some(x), Some(y)) => Some((x.clone(), y.clone())),
            (None, None) => None,
            // MG: Prefer unreachable! here.
            _ => panic!("The pool array is somehow non-square!"),
        }
    }

    fn record_bout(
        &mut self,
        fencer_left: usize,
        fencer_right: usize,
        score_left: PoolScore,
        score_right: PoolScore,
    ) {
        // Assumes indices are within bounds, will panic otherwise
        if fencer_left == fencer_right {
            // MG: prefer unreachable! here.
            panic!("A fencer cannot fence themselves!")
        }

        // MG: This is icky...
        // MG: Part of the reason I prefer a HashMap is because this just turns into two HashMap insert() calls...

        // Put the new scores in
        *self.get_cell_mut(fencer_left, fencer_right).unwrap() = PoolEntry::Complete(score_left);
        *self.get_cell_mut(fencer_right, fencer_left).unwrap() = PoolEntry::Complete(score_right);
    }

    fn is_complete(&self) -> bool {
      // MG: Part of why I prefer a HashMap is because all u need to do is check HashMap's len is n*(n-1)
      
        for row in self.scores.iter() {
            for score in row.iter() {
                if let PoolEntry::NotYet = score {
                    return false;
                }
            }
        }
        return true;
    }

    fn victories(&self, fencer: usize) -> u32 {
      // MG: Replace all these usize's with FencerID 
        self.scores[fencer]
            .iter()
            .filter(|score| score.for_scoring().victory)
            .count() as u32
    }

    fn touches_scored(&self, fencer: usize) -> u32 {
        self.scores[fencer]
            .iter()
            .map(|score| score.for_scoring().touches)
            .sum()
    }

    fn touches_received(&self, fencer: usize) -> u32 {
        self.scores
            .iter()
            .map(|row| &row[fencer])
            .map(|score| score.for_scoring().touches)
            .sum()
    }

    fn indicator(&self, fencer: usize) -> i32 {
        let ts = self.touches_scored(fencer) as i32;
        let tr = self.touches_received(fencer) as i32;
        return ts - tr;
    }
}

struct Tournament {
    // Contains all the information on a tournament.
    // TODO: right now Pools index into the "fencers" field, because they can't
    // hold a reference to it. This is, how you say, Bad, I think. 
    
    // MG: Not necessarily. re:^ Sometimes it's easier to do that rather than do self-borrowing.

    // MG: Make this into a HashMap<FencerId, FencerData>
    fencers: Vec<FencerData>,

    // MG: Why not just have a Vec<Pool> that is empty?
    pools: Option<Vec<Pool>>,
}

impl Tournament {
    fn new(fencers: Vec<FencerData>) -> Tournament {
        Tournament {
            fencers,
            pools: None,
        }
    }

    // MG: Why not do this in the constructor? Tournament takes fencers and num pools?
    fn create_pools(&mut self, num_pools: usize) {
        // this isn't the correct way to distribute the fencers, but it'll do for now

        let mut groups: Vec<Vec<FencerID>> = vec![vec![]; num_pools];

        // MG: If u use a real hashmap for fencers, then u can change 0..self.fencers.len() into self.fencers.keys() :P
        for i in 0..self.fencers.len() {
            groups[i % num_pools].push(i as FencerID);
        }
        
        let pools = groups.into_iter().map(Pool::new).collect();
        self.pools = Some(pools)
    }

    // MG: Then this should fail always if pools is not an Option but just a Vec
    fn get_pool_mut(&mut self, pool: usize) -> Option<&mut Pool> {
        match self.pools {
            Some(ref mut pools) => pools.get_mut(pool),
            None => None,
        }
    }
}

// --- testing stuff ---

extern crate prettytable;
use prettytable::{Cell, Row, Table};

fn prettyprintpool(tournament: &Tournament, pool: &Pool) {
    let mut table = Table::new();

    // type bound means it should be able to take &String and &str
    fn make_row<T: AsRef<str>>(items: &[T]) -> Row {
        let cells: Vec<Cell> = items.iter().map(|x| Cell::new(x.as_ref())).collect();
        Row::new(cells)
    }

    // Add first row
    let mut headers = vec!["Name"];

    for fencer_id in pool.fencers.iter().copied() {
        let fencer_name = &tournament.fencers[fencer_id].name;
        // Get first two characters
        match fencer_name.char_indices().nth(2) {
            Some((idx, _)) => headers.push(&fencer_name[0..idx]),
            None => headers.push(&fencer_name),
        }
    }
    headers.extend(vec!["V", "TS", "TR", "Ind"]);
    table.add_row(make_row(&headers));

    // Add other rows
    for (i, fencer_id) in pool.fencers.iter().copied().enumerate() {
        let fencer = &tournament.fencers[fencer_id];
        let mut row = vec![fencer.name.clone()];

        for j in 0..pool.size() {
            let score = pool.get_cell(i, j);
            let text = match score.unwrap() {
                PoolEntry::Complete(score) => {
                    let prefix = if score.victory { "V" } else { "D" };
                    format!("{}{}", prefix, score.touches)
                }
                PoolEntry::NotYet => String::from(""),
                PoolEntry::SameFencer => String::from("--"),
            };
            row.push(text);
        }

        // Add the stat columns at the end
        row.push(pool.victories(i).to_string());
        row.push(pool.touches_scored(i).to_string());
        row.push(pool.touches_received(i).to_string());
        row.push(pool.indicator(i).to_string());

        table.add_row(make_row(&row));
    }

    table.printstd();
}

fn main() {
    let names = ["Alice", "Bob", "Charlie", "Dolly"];
    let fencers: Vec<FencerData> = names.into_iter().map(|s| FencerData::new(s)).collect();

    let mut tournament = Tournament::new(fencers);
    tournament.create_pools(1);

    if let Some(pool) = tournament.get_pool_mut(0) {
        pool.record_bout(
            0,
            1,
            PoolScore {
                touches: 5,
                victory: true,
            },
            PoolScore {
                touches: 2,
                victory: false,
            },
        );
        pool.record_bout(
            2,
            3,
            PoolScore {
                touches: 4,
                victory: false,
            },
            PoolScore {
                touches: 4,
                victory: true,
            },
        );
        pool.record_bout(
            0,
            2,
            PoolScore {
                touches: 0,
                victory: false,
            },
            PoolScore {
                touches: 3,
                victory: true,
            },
        );
    }

    for pool in tournament.pools.as_ref().unwrap().iter() {
        prettyprintpool(&tournament, pool);
    }
}