Code review of my small data collector

Good evening everybody,

today I finished my first small Rust program after reading the "the book" a few months ago, forgetting all about it afterwards and just taking the plunge a few days ago (bear with me please).
So as a disclaimer: I know my program is programmatically very sloppy with a lot things not considered in the first place, but I plan on adding to it every now and then from now on and would love some valuable feedback regarding best practices and such.

I wrote a small tool scraping basic data from binance.com using binance-rs and writing it to an InfluxDB instance using reqwest, all of the code fits within 200 lines:

#[macro_use] extern crate log;
extern crate simplelog;

use binance::{api::Binance, errors::{Error, ErrorKind::BinanceError}, market::Market};
use figment::{Figment, providers::{Format, Toml}};
use serde::Deserialize;
use simplelog::*;
use std::collections::HashMap;
use std::fs::File;
use std::thread;
use std::time::{Duration, SystemTime, SystemTimeError};


#[derive(Deserialize)]
struct GeneralSettings {
    query_interval: u64,
    currency: String,
    log: String,
}

#[derive(Deserialize)]
struct Currency {
    amount: f64,
    price: f64,
}

#[derive(Deserialize)]
struct InfluxDB {
    host: String,
    port: Option<u16>,
    token: String,
    org: String,
    bucket: String,
}

#[derive(Deserialize)]
struct Config {
    general_settings: GeneralSettings,
    influxdb: InfluxDB,
    wallet: HashMap<String, Currency>,
}


fn query_currency(currency: &str, market: &Market, symbol: &str) -> f64 {
    let mut owned_currency = symbol.to_owned();
    owned_currency.push_str(currency);
    match market.get_price(owned_currency) {
        Ok(response) => response.price,
        Err(Error(BinanceError(e), _)) => {
            if e.code == -1121 {
                owned_currency = symbol.to_owned();
                owned_currency.push_str("USDT");
                match market.get_price(owned_currency) {
                    Ok(response) => {
                        let eur_usd = match market.get_price("EURUSDT") {
                            Ok(response) => response.price, 
                            Err(_) => {
                                warn!("An error occured while trying to get price data for EUR->USDT: {:?}", e);
                                -1.0
                            },
                        };
                        
                        response.price / eur_usd
                    },
                    Err(_) => {
                        warn!("An error occured while trying to get price data for {}->{}: {:?}", currency, symbol, e);
                        -1.0
                    },
                }
            } else {
                warn!("An error occured while trying to get price data for {}->{}: {:?}", currency, symbol, e);
                -1.0
            }
        },
        Err(e) => {
            error!("An unexpected error occured while trying to get price data for {}->{}: {:?}", currency, symbol, e);
            -1.0
        },
    }
}


fn build_line_protocol(price_data: &HashMap<String, f64>,
                       change_data: &HashMap<String, f64>,
                       gains_data: &HashMap<String, f64>) -> Result<String, SystemTimeError> {
    let mut line_protocol = String::new();
    let time_stamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH)?.as_secs();
    for key in price_data.keys() {
        line_protocol.push_str(&format!("crypto_data,currency={} price={},change={},net={} {}\n", key, price_data[key], change_data[key], gains_data[key], time_stamp))
    }
    Ok(line_protocol)
}


fn send_to_influxdb(host: &str,
                    port: u16,
                    token: &str,
                    org: &str,
                    bucket: &str,
                    body: String) -> Result<(), reqwest::Error> {
    let mut secret_token = String::from("Token ");
    secret_token.push_str(token);
    let secret_token = Box::leak(secret_token.into_boxed_str());
    let mut secret_token = reqwest::header::HeaderValue::from_static(secret_token);
    secret_token.set_sensitive(true);
    let mut headers = reqwest::header::HeaderMap::new();
    headers.insert("Content-Type", reqwest::header::HeaderValue::from_static("text/plain; charset=utf-8"));
    headers.insert("Accept", reqwest::header::HeaderValue::from_static("application/json"));
    headers.insert(reqwest::header::AUTHORIZATION, secret_token);

    let client = reqwest::blocking::Client::builder()
        .default_headers(headers)
        .build()?;

    let mut url = String::from(host);
    url.push_str(&format!(":{}/api/v2/write?org={}&bucket={}&precision=s", port, org, bucket));
    
    let _response = client.post(url)
        .body(body)
        .send()?;

    return Ok(());
}


fn run(config: Config) -> Result<(), Box<dyn std::error::Error>> {
    let market: Market = Binance::new(None, None); 
    let mut price_data: HashMap<String, f64> = HashMap::new();
    let mut change_data: HashMap<String, f64> = HashMap::new();
    let mut gains_data: HashMap<String, f64> = HashMap::new();
    let mut change: f64;

    loop {
        for currency in config.wallet.keys() {
            let price = query_currency(&config.general_settings.currency, &market, currency);

            if price != -1.0 {
                price_data.insert(currency.to_string(), price);
                change = price / config.wallet[currency].price - 1.0;
                change_data.insert(currency.to_string(), change);
                gains_data.insert(currency.to_string(), config.wallet[currency].amount * change * config.wallet[currency].price);
            } else {
                warn!("Couldn't fetch price for {}, skipping...", currency);
            }
        }

        match build_line_protocol(&price_data, &change_data, &gains_data) {
            Ok(body) => {
                match send_to_influxdb(&config.influxdb.host,
                                 config.influxdb.port.unwrap(),
                                 &config.influxdb.token,
                                 &config.influxdb.org,
                                 &config.influxdb.bucket,
                                 body) {
                    Ok(_) => info!("Sent data to InfluxDB successfully."),
                    Err(e) => error!("An error occured while trying to send data to InfluxDB: {}", e),
                }
            },
            Err(e) => error!("An error occured while trying to get UNIX timestamp for this iteration: {}", e),
        }

        price_data.clear();
        change_data.clear();
        gains_data.clear();
        thread::sleep(Duration::from_secs(config.general_settings.query_interval));
    }
}


fn main() -> Result<(), Box<dyn std::error::Error>> {
    let mut config: Config = Figment::new()
        .merge(Toml::file("/Users/daniel/repositories/crypto/config.toml"))
        .extract()?;

    if config.influxdb.port == None {
        config.influxdb.port = Some(8086);
    }

    CombinedLogger::init(
        vec![
            TermLogger::new(LevelFilter::Warn, simplelog::Config::default(), TerminalMode::Mixed, ColorChoice::Auto),
            WriteLogger::new(LevelFilter::Info, simplelog::Config::default(), File::create(&config.general_settings.log).unwrap()),
        ]
    ).unwrap();

   run(config) 
}

If needed I can provide an example configuration file or a link to my (private) repository as well..

Cheers and thanks in advance,
all the best,
Daniel

I suggest using Results instead of sentinel values. Here's a mock-up of how query_currency might look if refactored to more of a get-next-piece-or-return flow (which doesn't technically need the Result return value):

fn query_currency(currency: &str, market: &Market, symbol: &str) -> Result<f64, &'static str> {
    let mut owned_currency = symbol.to_owned();
    owned_currency.push_str(currency);
    
    let e = match market.get_price(owned_currency) {
        Err(Error(BinanceError(e), _)) => e,
        Err(e) => return Err(""),
        Ok(response) => return Ok(response.price),
    };
    
    if e.code != -1121 {
        return Err("");
    }
    
    owned_currency = symbol.to_owned();
    owned_currency.push_str("USDT");
    let response = market.get_price(owned_currency).map_err(|e| "")?;
    let eur_usd = market.get_price("EURUSDT").map_err(|e| "")?;

    Ok(response.price / eur_usd.price)
}

(Then the next step would probably be a custom error type with a From implementation.)

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.