Option Spaghetti

[jbradenbrown]

I'm playing around with a data structure for holding a couple of different types and I'm creating a gross Option spaghetti. I've just started learning Rust, and I think there is probably a better way to do this that I am not aware of. There is also a lot of code duplication that I don't know how to get rid of. Any help would be appreciated.

Playground

use chrono::NaiveDateTime;
use std::collections::HashMap;
use std::ops::Index;

#[derive(Debug)]
pub enum Column {
    Numeric(Box<Vec<Option<f64>>>),
    Alphabetic(Box<Vec<Option<String>>>),
    Tempic(Box<Vec<Option<NaiveDateTime>>>),
}

impl Column {
    fn numeric() -> Column {
        Column::Numeric(Box::new(Vec::new()))
    }

    fn alphabetic() -> Column {
        Column::Alphabetic(Box::new(Vec::new()))
    }

    fn tempic() -> Column {
        Column::Tempic(Box::new(Vec::new()))
    }

    fn unwrap_numeric(&self) -> Option<&Box<Vec<Option<f64>>>> {
        if let Column::Numeric(v) = self {
            return Some(v);
        } else {
            return None;
        }
    }

    fn unwrap_alphabetic(&self) -> Option<&Box<Vec<Option<String>>>> {
        if let Column::Alphabetic(v) = self {
            return Some(v);
        } else {
            return None;
        }
    }

    fn unwrap_tempic(&self) -> Option<&Box<Vec<Option<NaiveDateTime>>>> {
        if let Column::Tempic(v) = self {
            return Some(v);
        } else {
            return None;
        }
    }
}

pub struct Tansu {
    pub data: HashMap<String, Column>,
}

impl Tansu {
    pub fn new() -> Tansu {
        Tansu {
            data: HashMap::new(),
        }
    }

    pub fn add_to_numeric(&mut self, key: &str, data: Option<f64>) {
        let c = self
            .data
            .entry(key.to_string())
            .or_insert(Column::numeric());
        if let Column::Numeric(v) = c {
            v.push(data);
        }
    }

    pub fn add_to_alphabetic(&mut self, key: &str, data: Option<String>) {
        let c = self
            .data
            .entry(key.to_string())
            .or_insert(Column::alphabetic());
        if let Column::Alphabetic(v) = c {
            v.push(data);
        }
    }

    pub fn add_to_tempic(&mut self, key: &str, data: Option<NaiveDateTime>) {
        let c = self.data.entry(key.to_string()).or_insert(Column::tempic());
        if let Column::Tempic(v) = c {
            v.push(data);
        }
    }
}

impl Index<&str> for Tansu {
    type Output = Column;

    fn index(&self, key: &str) -> &Self::Output {
        match &self.data.get(key) {
            Some(r) => r,
            None => panic!(),
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    fn setup() -> Tansu {
        Tansu::new()
    }

    #[test]
    fn no_existing_key() {
        let mut t = setup();

        t.add_to_numeric("test", Some(5.0));

        assert_eq!(
            t.data.get("test").unwrap().unwrap_numeric().unwrap()[0],
            Some(5.0)
        );
    }

    #[test]
    fn indexable() {
        let mut t = setup();

        t.add_to_numeric("test", Some(5.0));

        assert_eq!(t["test"].unwrap_numeric().unwrap()[0], Some(5.0));
    }

    #[test]
    fn print_it() {
        println!("{:?}", "hi");
    }

}

[alice]

Okay so:

1. You do not need to use Box inside Column when the immediate child is a container like Vec or HashMap etc.
2. Typically we recommend returning a slice instead of a &Vec, since the latter provides no extra functionality, but is less general. This simplifies the unwrap_* methods to Option<&[Option<f64>]>.
3. Note that it is possible to return an empty list when you unwrapped the wrong type. Simply return &[] in this case and use the return type &[Option<f64>].
4. Is it a requirement that no column has stuff of different types? Currently you silently ignore it if you add a cell with a different type from a different cell at the same string key.
5. Your add_to_* methods currently take a &str for the key, but they always convert it to a String. The recommendation is that you should take a String if you'd otherwise always copy it. If having to convert "" literals into Strings is annoying, you can take an impl Into<String> instead.

You may find this playground useful.

[jbradenbrown]

Got rid of the boxes, changed to slices, and swapped some of the options for results since that seemed like it was more descriptive.
Is there something that I can do to collapse the add_to_* and unwrap_* methods without giving up the guarantee that each column has only a single type?
I suppose I could use a macro to create functions like that?

Playground

[asymmetrikon]

Yes, you can do those with macros:

macro_rules! unwrap_impl {
    ($self:ident, $var:ident, $name:literal) => {
        if let Column::$var(v) = $self {
            Ok(v)
        } else {
            Err(Error::new(ErrorKind::NotFound, concat!("column is not ", $name)))
        }
    };
}

// example 
fn unwrap_numeric(&self) -> Result<&[Option<f64>]> {
    unwrap_impl!(self, Numeric, "numeric")
}

macro_rules! add_to_impl {
    ($self:ident, $key:expr, $data:expr, $def:expr, $var:ident, $name:literal) => {{
        let c = $self.data
            .entry($key.to_string())
            .or_insert($def);
        
        unwrap_impl!(c, $var, $name)
            .map(|v| { v.push($data); })
            .map_err(|_| Error::new(ErrorKind::NotFound, format!(concat!("column with key {} is not ", $name), $key)))
    }};
}

// example
pub fn add_to_numeric(&mut self, key: &str, data: Option<f64>) -> Result<()> {
    add_to_impl!(self, key, data, Column::numeric(), Numeric, "numeric")
}

Though I'd say that your unwrap_* names are more idiomatically named as_* (like as_numeric.)

[Michael-F-Bryan]

I've found the if_chain crate works well when you need to convert a bunch of nested if Let(...) statements or and_then()s.

if_chain! {
    if let Some(y) = x;
    if let Some(z) = y;
    then {
        do_stuff_with(z)
    } else {
        do_something_else()
    }
}

// becomes

if let Some(y) = x {
    if let Some(z) = y {
        do_stuff_with(z)
    } else {
        do_something_else()
    }
} else {
    do_something_else()
}

[17cupsofcoffee]

An eRFC has been accepted to integrate a similar syntax into the language, which would make your example look something like this:

if let Some(y) = x && let Some(z) = y {
    do_stuff_with(z)
} else {
    do_something_else()
}

The exact syntax isn't final yet, though.