earth-metabolome-initiative / emi-monorepo

Monorepo for the Earth Metabolome Initiative
GNU General Public License v3.0
5 stars 0 forks source link

A 'lifetime question ... #1

Closed oolonek closed 7 months ago

oolonek commented 7 months ago

... we had with Marco.

here

impl<'a> TryFrom<&'a str> for FormulaSearchDB {
    type Error = String;

    fn try_from(s: &'a str) -> Result<Self, Self::Error> {
        match s {
            "BIO" => Ok(FormulaSearchDB::Bio),
            "METACYC" => Ok(FormulaSearchDB::Metacyc),
            "CHEBI" => Ok(FormulaSearchDB::Chebi),
            "COCONUT" => Ok(FormulaSearchDB::Coconut),
            "ECOCYCMINE" => Ok(FormulaSearchDB::Ecocycmine),
            "GNPS" => Ok(FormulaSearchDB::Gnps),
            "HMDB" => Ok(FormulaSearchDB::Hmdb),
            "HSDB" => Ok(FormulaSearchDB::Hsdb),
            "KEGG" => Ok(FormulaSearchDB::Kegg),
            "KEGGMINE" => Ok(FormulaSearchDB::Keggmine),
            "KNAPSACK" => Ok(FormulaSearchDB::Knapsack),
            "MACONDA" => Ok(FormulaSearchDB::Maconda),
            "MESH" => Ok(FormulaSearchDB::Mesh),
            "NORMAN" => Ok(FormulaSearchDB::Norman),
            "UNDP" => Ok(FormulaSearchDB::Undp),
            "PLANTCYC" => Ok(FormulaSearchDB::Plantcyc),
            "PUBCHEM" => Ok(FormulaSearchDB::Pubchem),
            "PUBMED" => Ok(FormulaSearchDB::Pubmed),
            "YMDB" => Ok(FormulaSearchDB::Ymdb),
            "YMDBMINE" => Ok(FormulaSearchDB::Ymdbmine),
            "ZINCBIO" => Ok(FormulaSearchDB::Zincbio),
            _ => Err(format!("Unknown formula search db: {}", s)),
        }
    }
}

impl TryFrom<String> for FormulaSearchDB {
    type Error = String;

    fn try_from(s: String) -> Result<Self, Self::Error> {
        FormulaSearchDB::try_from(s.as_str())
    }
}

Could you explain us the purpose of this "double" implementation of TryFrom (one with the lifetime and ref the other with the object itself) ?

LucaCappelletti94 commented 7 months ago

It is primarily for ease of use. Let's start by breaking down what the different string objects mean:

oolonek commented 7 months ago

So if I get this right, the second block is somehow redundant ? We could just keep the first one since we can get the the underlying str from a String Struct for &'a str? Or is there a reason to explicity implement it ?

LucaCappelletti94 commented 7 months ago

It is primarily a clean code thing. This way, a library user can use this ready made method without having to duplicate that logic several times.