Rustacean way to parse a date from &str [Rust](DRY)

code review
#1

I have this function, but I feel that it's duplicate code. I was wondering if anyone here shares how to make it more rustacean. I'm still learning Rust, and I thought this could be a good example to share.

fn check_and_transform_dates(start_date: &str, end_date: &str) -> (i64, i64) {
    let message: String = format!(
        "Data could not be downloaded ❌, please make sure your dates
    are in the following format YYYY-MM-DD
    (ie. 2020-01-01), your dates are Start Date: {}, End Date: {}",
        &start_date, &end_date,
    );
    let start_date_parsed: i64 = Utc
        .ymd(
            FromStr::from_str(start_date.split('-').collect::<Vec<&str>>()[0]).unwrap_or_else(
                |_| {
                    eprintln!("{}", &message);
                    process::exit(1);
                },
            ),
            FromStr::from_str(start_date.split('-').collect::<Vec<&str>>()[1]).unwrap_or_else(
                |_| {
                    eprintln!("{}", &message);
                    process::exit(1);
                },
            ),
            FromStr::from_str(start_date.split('-').collect::<Vec<&str>>()[2]).unwrap_or_else(
                |_| {
                    eprintln!("{}", &message);
                    process::exit(1);
                },
            ),
        )
        .and_hms_milli(0, 0, 1, 0)
        .timestamp_millis()
        .clamp(
            Utc.ymd(2016, 1, 1)
                .and_hms_milli(0, 0, 0, 0)
                .timestamp_millis(),
            Utc::now().timestamp_millis(),
        );
    let end_date_parsed: i64 = Utc
        .ymd(
            FromStr::from_str(end_date.split('-').collect::<Vec<&str>>()[0]).unwrap_or_else(|_| {
                eprintln!("{}", &message);
                process::exit(1);
            }),
            FromStr::from_str(end_date.split('-').collect::<Vec<&str>>()[1]).unwrap_or_else(|_| {
                eprintln!("{}", &message);
                process::exit(1);
            }),
            FromStr::from_str(end_date.split('-').collect::<Vec<&str>>()[2]).unwrap_or_else(|_| {
                eprintln!("{}", &message);
                process::exit(1);
            }),
        )
        .and_hms_milli(0, 0, 2, 0)
        .timestamp_millis()
        .clamp(
            Utc.ymd(2016, 1, 1)
                .and_hms_milli(0, 0, 0, 0)
                .timestamp_millis(),
            Utc::now().timestamp_millis(),
        );

    (start_date_parsed, end_date_parsed)

Mainly to clean the three arguments passed to Utc.ymd since they are doing the same, just using a different index, they are parsing dates such as "2021-01-01" and returning it in milliseconds and clamping it to a floor and ceiling.

#2

One thing is to get rid of some Vec collecting by replacing the let start_date_parsed statment with something like this:

let parts = start_date.split('-');
let start_date_parsed: i64 = Utc
        .ymd(
            parts.next().and_then(FromStr::from_str).unwrap_or_else(exit_message),
            parts.next().and_then(FromStr::from_str).unwrap_or_else(exit_message),
            parts.next().and_then(FromStr::from_str).unwrap_or_else(exit_message),
        )

(Where exit_message is your closure writing a message and exiting the process, but declared as a function).

A second thing is that the date part is parsed the same way for the start date and the end date, so you could extract that part into a method, leaving the first function something like:

fn check_and_transform_dates(start_date: &str, end_date: &str) -> (i64, i64) {
    let message: String = format!(
        "Data could not be downloaded ❌, please make sure your dates
    are in the following format YYYY-MM-DD
    (ie. 2020-01-01), your dates are Start Date: {}, End Date: {}",
        &start_date, &end_date,
    );
    let start_date_parsed: i64 = parse_ymd(start_date)
        .and_hms_milli(0, 0, 1, 0)
        .timestamp_millis()
        .clamp(
            Utc.ymd(2016, 1, 1)
                .and_hms_milli(0, 0, 0, 0)
                .timestamp_millis(),
            Utc::now().timestamp_millis(),
        );
    let end_date_parsed: i64 = parse_ymd(end_date)
        .and_hms_milli(0, 0, 2, 0)
        .timestamp_millis()
        .clamp(
            Utc.ymd(2016, 1, 1)
                .and_hms_milli(0, 0, 0, 0)
                .timestamp_millis(),
            Utc::now().timestamp_millis(),
        );

    (start_date_parsed, end_date_parsed)
}

Next, the clamp timestamp are the same in both cases, so maybe declare those as variables before parsing the dates, so their code don't have to be repated.

I made a Rust Playground where I tested my suggestions. Some of the above don't really work quite as written, but the playground does, and I hope you can see how I got from the code you posted to there.

#3

If it's DRY you're after, why write the parsing code yourself when chrono can do it for you?

1 Like
#4

Is there a specific reason you are specifing the types for each variable? You don't have to. You can just write let message = format!(...), no need for : String.

Instead of Fromstr use parse.

Instead of panicking, you could return a Result :wink:

Mainly to remove the three arguments passed

then why don't you put it into an extra function?

Here's my attempt on solving your problem:

use chrono::{Date, TimeZone, Utc};

pub enum ErrorType {
    InvalidFormat,
    InvalidNumber,
}

fn create_date(date: &str) -> Result<Date<Utc>, ErrorType> {
    let mut dates = date.split('-');
    Ok(Utc.ymd(
        dates
            .next()
            .ok_or(ErrorType::InvalidFormat)?
            .parse()
            .map_err(|_| ErrorType::InvalidNumber)?,
        dates
            .next()
            .ok_or(ErrorType::InvalidFormat)?
            .parse()
            .map_err(|_| ErrorType::InvalidNumber)?,
        dates
            .next()
            .ok_or(ErrorType::InvalidFormat)?
            .parse()
            .map_err(|_| ErrorType::InvalidNumber)?,
    ))
}

pub fn check_and_transform_dates(
    start_date: &str,
    end_date: &str,
) -> Result<(i64, i64), ErrorType> {
    let start_date_parsed = create_date(start_date)?
        .and_hms_milli(0, 0, 1, 0)
        .timestamp_millis()
        .clamp(
            Utc.ymd(2016, 1, 1)
                .and_hms_milli(0, 0, 0, 0)
                .timestamp_millis(),
            Utc::now().timestamp_millis(),
        );
    let end_date_parsed = create_date(end_date)?
        .and_hms_milli(0, 0, 2, 0)
        .timestamp_millis()
        .clamp(
            Utc.ymd(2016, 1, 1)
                .and_hms_milli(0, 0, 0, 0)
                .timestamp_millis(),
            Utc::now().timestamp_millis(),
        );

    Ok((start_date_parsed, end_date_parsed))
}

Of course you can put more common pieces into the function. I see that your start and end date only differs by one minute, so you could also accept a minute argument and the rest would be equal as well.

If you have further questions, please feel free to ask them

2 Likes
#5

Or this.

fn create_date(date: &str) -> chrono::ParseResult<Date<Utc>> {
    Ok(Date::from_utc(date.parse()?, Utc))
}
2 Likes
#6

Here's my attempt, which pulls the common logic into a closure that gets reused:

use chrono::{NaiveDate,Utc}; // 0.4.19

fn check_and_transform_dates(start_date: &str, end_date: &str) -> (i64, i64) {
    let message: String = format!(
        "Data could not be downloaded ❌, please make sure your dates
    are in the following format YYYY-MM-DD
    (ie. 2020-01-01), your dates are Start Date: {}, End Date: {}",
        &start_date, &end_date,
    );
    
    let earliest = NaiveDate::from_ymd(2016,1,1);
    let today = Utc::today().naive_utc();
    
    let parse_date = |date: &str|->NaiveDate {
        let date = NaiveDate::parse_from_str(date, "%F").expect(&message);
        if date < earliest { earliest.clone() }
        else if date > today { today.clone() }
        else { date }
    };
    
    (
        parse_date(start_date).and_hms(0,0,1).timestamp() * 1000,
        parse_date(end_date).and_hms(0,0,2).timestamp() * 1000
    )
}

(Playground)

NB: This isn't exactly equivalent to your code: In my version, the time of day is added after clamping the date

1 Like
#7

I suspect that the exercise is learning the language here, not using a pre-existing crate :slight_smile:
So yes, your solution makes more sense in the long-term run.

#8

This works! Just used an expect_or_else(|_|{}); rather than expect, and you can remove the `clone()', since clippy will issue a warning

#9

It's an exercise, no need to depend on a library when you're learning, the abstraction makes you forget easy patterns that don't depend on some other crates.

#10

Here's the final function I'm using, I've selected it because it's a good pattern, and uses closures as well.

fn check_and_transform_dates(start_date: &str, end_date: &str) -> (i64, i64) {
    let message: String = format!(
        "Data could not be downloaded ❌, please make sure your dates
    are in the following format YYYY-MM-DD
    (ie. 2020-01-01), your dates are Start Date: {}, End Date: {}",
        &start_date, &end_date,
    );

    let earliest: NaiveDate = NaiveDate::from_ymd(2016, 1, 1);
    let today: NaiveDate = Utc::today().naive_utc();

    let parse_date = |date: &str| -> NaiveDate {
        let date: NaiveDate = NaiveDate::parse_from_str(date, "%F").unwrap_or_else(|_| {
            eprintln!("{}", &message);
            process::exit(1);
        });
        if date < earliest {
            earliest
        } else if date > today {
            today
        } else {
            date
        }
    };

    (
        parse_date(start_date).and_hms(0, 0, 1).timestamp() * 1000,
        parse_date(end_date).and_hms(0, 0, 2).timestamp() * 1000,
    )
}
#11

As already said: please avoid putting the type of the variable onto every declaration. You don't have to and it's not very useful to do so. Just write let earliest = NaiveDate::from_ymd(2016, 1, 1); and so on. Don't make it harder for yourself :wink:

1 Like
#12

@hellow

Yes, it's just a force of habit when writing