You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Custom features should be simplified, which requires some design improvements. There are also some minor coding issues.
design
These design issues we need to chat about. It'll be easier when it's clear what the config settings are for.
Rename ConfParser to CustomFeatureParser unless other settings can be loaded other than features (in which case, let's chat about it);
Why is there a FeatureSettings and RawFeatureSettings? Also, why are they plural? Moreover, there is no information about what the different fields in the conf file mean and how they affect FastaData.
get_conf_data takes a FastaData const&, copies it, adds the returns and returns it. Why not just pass a non-const reference in? Or better, have get_conf_data return a list of Feature's that can be added to FastaData after it returns. In the latter case, get_conf_data can be seen as a function that translates the config domain models (FeatureSettings) into the fasta parsing domain models. In reality they both share the concept of a Feature (among others) so perhaps we can factor out these common models.
code
In get_conf_data, seq.seq_no should probably always be >= 0 so I'd check this during parsing. Also, the warning message states that it should be between 1 and sequence length (not equal to zero);
In get_conf_data, don't cast size_t to signed, they aren't always the same. Instead cast the positions parsed from the config file to size_t;
Use static_cast<T> instead of c style casts as they are much safer;
Custom features should be simplified, which requires some design improvements. There are also some minor coding issues.
design
These design issues we need to chat about. It'll be easier when it's clear what the config settings are for.
ConfParsertoCustomFeatureParserunless other settings can be loaded other than features (in which case, let's chat about it);FeatureSettingsandRawFeatureSettings? Also, why are they plural? Moreover, there is no information about what the different fields in the conf file mean and how they affectFastaData.get_conf_datatakes aFastaData const&, copies it, adds the returns and returns it. Why not just pass a non-const reference in? Or better, haveget_conf_datareturn a list ofFeature's that can be added toFastaDataafter it returns. In the latter case,get_conf_datacan be seen as a function that translates the config domain models (FeatureSettings) into the fasta parsing domain models. In reality they both share the concept of aFeature(among others) so perhaps we can factor out these common models.code
get_conf_data,seq.seq_noshould probably always be>= 0so I'd check this during parsing. Also, the warning message states that it should be between 1 and sequence length (not equal to zero);get_conf_data, don't castsize_ttosigned, they aren't always the same. Instead cast the positions parsed from the config file tosize_t;static_cast<T>instead of c style casts as they are much safer;