Skip to content

Commit

Permalink
move initialization of OptionInt/Double parameters to constructor (#1364
Browse files Browse the repository at this point in the history
)

* move init of OptionInt/Double parms to ctor

except for id, which isn't known by formats/filters.
Also, move validation of OptionInt/Double strings to Option
class, where it can be maintained in parallel with the conversion
routines.

* init Option id in init_vec, init_filter_vec.

also
fix -Wclazy-unused-non-trivial-variable.
add test of Option error detection.
  • Loading branch information
tsteven4 authored Oct 31, 2024
1 parent cadbc6a commit 21603ae
Show file tree
Hide file tree
Showing 23 changed files with 124 additions and 117 deletions.
6 changes: 3 additions & 3 deletions arcdist.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ArcDistanceFilter:public Filter
/* Data Members */

double pos_dist{};
OptionDouble distopt;
OptionDouble distopt{true};
OptionString arcfileopt;
OptionBool rteopt;
OptionBool trkopt;
Expand All @@ -71,7 +71,7 @@ class ArcDistanceFilter:public Filter

QVector<arglist_t> args = {
{
"file", &arcfileopt, "File containing vertices of arc",
"file", &arcfileopt, "File containing vertices of arc",
nullptr, ARGTYPE_FILE, ARG_NOMINMAX, nullptr
},
{
Expand All @@ -84,7 +84,7 @@ class ArcDistanceFilter:public Filter
},
{
"distance", &distopt, "Maximum distance from arc",
nullptr, ARGTYPE_ALLOW_TRAILING_DATA | ARGTYPE_STRING | ARGTYPE_REQUIRED, ARG_NOMINMAX, nullptr
nullptr, ARGTYPE_STRING | ARGTYPE_REQUIRED, ARG_NOMINMAX, nullptr
},
{
"exclude", &exclopt, "Exclude points close to the arc", nullptr,
Expand Down
10 changes: 0 additions & 10 deletions defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -866,18 +866,8 @@ extern posn_status tracking_status;
* for "groups" of exactly one option. */
#define ARGTYPE_BEGIN_REQ 0x04000000U
#define ARGTYPE_END_REQ 0x02000000U
/* For integer conversions specify the base to allow strict error checking and
* proper conversion in Vecs */
#define ARGTYPE_BASE_10 0x00000000U
#define ARGTYPE_BASE_AUTO 0x00100000U
#define ARGTYPE_BASE_16 0x00200000U

/* For integer and double conversions is trailing data allowed?
* This allows strict error checking in Vecs */
#define ARGTYPE_ALLOW_TRAILING_DATA 0x00400000U

#define ARGTYPE_TYPEMASK 0x00000fffU
#define ARGTYPE_BASEMASK 0x00300000U
#define ARGTYPE_FLAGMASK 0xfffff000U

#define ARG_NOMINMAX nullptr, nullptr
Expand Down
11 changes: 9 additions & 2 deletions filter_vecs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -270,19 +270,26 @@ void FilterVecs::free_filter_vec(Filter* flt)
Vecs::free_options(flt->get_args());
}

void FilterVecs::init_filter_vec(Filter* flt)
void FilterVecs::init_filter_vec(Filter* flt, const QString& fltname)
{
QVector<arglist_t>* args = flt->get_args();
if (args && !args->isEmpty()) {
assert(args->isDetached());
for (auto& arg : *args) {
if (arg.argval != nullptr) {
arg.argval->reset();
QString id = QStringLiteral("%1(%2)").arg(fltname, arg.argstring);
arg.argval->init(id);
}
}
}
}

void FilterVecs::init_filter_vecs()
{
for (const auto& vec : d_ptr_->filter_vec_list) {
if (vec.vec != nullptr) {
init_filter_vec(vec.vec);
init_filter_vec(vec.vec, vec.name);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion filter_vecs.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class FilterVecs
static void prepare_filter(const fltinfo_t& fltdata);
fltinfo_t find_filter_vec(const QString& fltargstring);
static void free_filter_vec(Filter* flt);
static void init_filter_vec(Filter* flt);
static void init_filter_vec(Filter* flt, const QString& fltname);
void init_filter_vecs();
static void exit_filter_vec(Filter* flt);
void exit_filter_vecs();
Expand Down
4 changes: 2 additions & 2 deletions garmin.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class GarminFormat : public Format
OptionBool snwhiteopt;
OptionString deficon;
OptionInt categoryopt;
OptionInt categorybitsopt;
OptionInt categorybitsopt{false, 0};
OptionInt baudopt;
OptionString opt_codec;
int baud = 0;
Expand Down Expand Up @@ -171,7 +171,7 @@ class GarminFormat : public Format
},
{
"bitscategory", &categorybitsopt, "Bitmap of categories",
nullptr, ARGTYPE_BASE_AUTO | ARGTYPE_INT, "1", "65535", nullptr
nullptr, ARGTYPE_INT, "1", "65535", nullptr
},
{
"baud", &baudopt, "Speed in bits per second of serial port (baud=9600)",
Expand Down
8 changes: 4 additions & 4 deletions garmin_gpi.h
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ class GarminGPIFormat : public Format
OptionBool opt_unique;
OptionBool opt_alerts;
OptionString opt_units;
OptionDouble opt_speed;
OptionDouble opt_proximity;
OptionDouble opt_speed{true};
OptionDouble opt_proximity{true};
OptionInt opt_sleep;
OptionString opt_lang;
OptionString opt_writecodec;
Expand Down Expand Up @@ -357,15 +357,15 @@ class GarminGPIFormat : public Format
},
{
"proximity", &opt_proximity, "Default proximity",
nullptr, ARGTYPE_ALLOW_TRAILING_DATA | ARGTYPE_STRING, ARG_NOMINMAX, nullptr
nullptr, ARGTYPE_STRING, ARG_NOMINMAX, nullptr
},
{
"sleep", &opt_sleep, "After output job done sleep n second(s)",
nullptr, ARGTYPE_INT, "1", nullptr, nullptr
},
{
"speed", &opt_speed, "Default speed",
nullptr, ARGTYPE_ALLOW_TRAILING_DATA | ARGTYPE_STRING, ARG_NOMINMAX, nullptr
nullptr, ARGTYPE_STRING, ARG_NOMINMAX, nullptr
},
{
"unique", &opt_unique, "Create unique waypoint names (default = yes)",
Expand Down
4 changes: 2 additions & 2 deletions gdb.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class GdbFormat : public Format
OptionInt gdb_opt_ver;
OptionBool gdb_opt_via;
OptionBool gdb_opt_roadbook;
OptionInt gdb_opt_bitcategory;
OptionInt gdb_opt_bitcategory{false, 0};
OptionBool gdb_opt_drop_hidden_wpt;

int waypt_flag{};
Expand All @@ -187,7 +187,7 @@ class GdbFormat : public Format
},
{
"bitscategory", &gdb_opt_bitcategory, "Bitmap of categories",
nullptr, ARGTYPE_BASE_AUTO | ARGTYPE_INT, "1", "65535", nullptr
nullptr, ARGTYPE_INT, "1", "65535", nullptr
},
{
"ver", &gdb_opt_ver,
Expand Down
4 changes: 2 additions & 2 deletions height.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class HeightFilter:public Filter
void process() override;

private:
OptionDouble addopt;
OptionDouble addopt{true};
OptionBool wgs84tomslopt;
double addf{};
// include static constexpr data member definitions with intializers for grid as private members.
Expand All @@ -56,7 +56,7 @@ class HeightFilter:public Filter
QVector<arglist_t> args = {
{
"add", &addopt, "Adds a constant value to every altitude",
nullptr, ARGTYPE_ALLOW_TRAILING_DATA | ARGTYPE_BEGIN_REQ | ARGTYPE_STRING, ARG_NOMINMAX, nullptr
nullptr, ARGTYPE_BEGIN_REQ | ARGTYPE_STRING, ARG_NOMINMAX, nullptr
},
{
"wgs84tomsl", &wgs84tomslopt, "Converts WGS84 ellipsoidal height to orthometric height (MSL)",
Expand Down
4 changes: 2 additions & 2 deletions interpolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class InterpolateFilter:public Filter

OptionDouble opt_time;
double max_time_step{0};
OptionDouble opt_dist;
OptionDouble opt_dist{true};
double max_dist_step{0};
OptionBool opt_route;

Expand All @@ -63,7 +63,7 @@ class InterpolateFilter:public Filter
},
{
"distance", &opt_dist, "Distance interval",
nullptr, ARGTYPE_END_EXCL | ARGTYPE_END_REQ | ARGTYPE_ALLOW_TRAILING_DATA | ARGTYPE_STRING,
nullptr, ARGTYPE_END_EXCL | ARGTYPE_END_REQ | ARGTYPE_STRING,
ARG_NOMINMAX, nullptr
},
{
Expand Down
10 changes: 5 additions & 5 deletions main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ run_reader(Vecs::fmtinfo_t& ivecs, const QString& fname)
start_session(ivecs.fmtname, fname);
if (ivecs.isDynamic()) {
ivecs.fmt = ivecs.factory(fname);
Vecs::init_vec(ivecs.fmt);
Vecs::init_vec(ivecs.fmt, ivecs.fmtname);
Vecs::prepare_format(ivecs);

ivecs->rd_init(fname);
Expand Down Expand Up @@ -277,7 +277,7 @@ run_writer(Vecs::fmtinfo_t& ovecs, const QString& ofname)
}
if (ovecs.isDynamic()) {
ovecs.fmt = ovecs.factory(ofname);
Vecs::init_vec(ovecs.fmt);
Vecs::init_vec(ovecs.fmt, ovecs.fmtname);
Vecs::prepare_format(ovecs);

ovecs->wr_init(ofname);
Expand Down Expand Up @@ -462,7 +462,7 @@ run(const char* prog_name)
}
if (filter.isDynamic()) {
filter.flt = filter.factory();
FilterVecs::init_filter_vec(filter.flt);
FilterVecs::init_filter_vec(filter.flt, filter.fltname);
FilterVecs::prepare_filter(filter);

filter->init();
Expand Down Expand Up @@ -650,11 +650,11 @@ run(const char* prog_name)

if (ivecs.isDynamic()) {
ivecs.fmt = ivecs.factory(fname);
Vecs::init_vec(ivecs.fmt);
Vecs::init_vec(ivecs.fmt, ivecs.fmtname);
}
if (ovecs && ovecs.isDynamic()) {
ovecs.fmt = ovecs.factory(ofname);
Vecs::init_vec(ovecs.fmt);
Vecs::init_vec(ovecs.fmt, ovecs.fmtname);
}

start_session(ivecs.fmtname, fname);
Expand Down
35 changes: 30 additions & 5 deletions option.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,9 @@ double OptionString::toDouble(bool* ok, QString* end) const
return parse_double(value_, id_, ok, end);
}

void OptionInt::init(const QString& id, bool allow_trailing_data, int base)
void OptionInt::init(const QString& id)
{
id_ = id;
base_ = base;
allow_trailing_data_ = allow_trailing_data;
}

void OptionInt::reset()
Expand All @@ -77,6 +75,15 @@ void OptionInt::set(const QString& s)
result_ = parse_integer(value_, id_, dieonerror, endp, base_);
}

bool OptionInt::isValid(const QString& s) const
{
bool ok;
QString end;
QString* endp = allow_trailing_data_? &end : nullptr;
(void) parse_integer(s, id_, &ok, endp, base_);
return ok;
}

int OptionInt::get_result(QString* end) const
{
if (end != nullptr) {
Expand All @@ -85,10 +92,14 @@ int OptionInt::get_result(QString* end) const
return result_;
}

void OptionDouble::init(const QString& id, bool allow_trailing_data, int /* base */)
bool OptionInt::trailing_data_allowed() const
{
return allow_trailing_data_;
}

void OptionDouble::init(const QString& id)
{
id_ = id;
allow_trailing_data_ = allow_trailing_data;
}

void OptionDouble::reset()
Expand All @@ -108,10 +119,24 @@ void OptionDouble::set(const QString& s)
result_ = parse_double(value_, id_, dieonerror, endp);
}

bool OptionDouble::isValid(const QString& s) const
{
bool ok;
QString end;
QString* endp = allow_trailing_data_? &end : nullptr;
(void) parse_double(s, id_, &ok, endp);
return ok;
}

double OptionDouble::get_result(QString* end) const
{
if (end != nullptr) {
*end = end_;
}
return result_;
}

bool OptionDouble::trailing_data_allowed() const
{
return allow_trailing_data_;
}
21 changes: 16 additions & 5 deletions option.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class Option /* Abstract Class */
[[nodiscard]] virtual bool has_value() const = 0;
[[nodiscard]] virtual bool isEmpty() const = 0;
[[nodiscard]] virtual const QString& get() const = 0;
virtual void init(const QString& id, bool allow_trailing_data, int base) {}
virtual void init(const QString& id) {}
virtual void reset() = 0;
virtual void set(const QString& s) = 0;

Expand Down Expand Up @@ -83,7 +83,7 @@ class OptionString : public Option
return value_;
}

void init(const QString& id, bool /* allow_trailing_data */, int /* base */) override
void init(const QString& id) override
{
id_ = id;
}
Expand Down Expand Up @@ -116,6 +116,10 @@ class OptionInt : public Option
public:
/* Special Member Functions */
OptionInt() = default;
explicit OptionInt(bool allow_trailing_data, int base) :
allow_trailing_data_(allow_trailing_data),
base_(base)
{}

explicit(false) operator const QString& () const
{
Expand All @@ -142,25 +146,30 @@ class OptionInt : public Option
return value_;
}

void init(const QString& id, bool allow_trailing_data, int base) override;
void init(const QString& id) override;
void reset() override;
void set(const QString& s) override;
bool isValid(const QString& s) const;
int get_result(QString* end = nullptr) const;
bool trailing_data_allowed() const;

private:
QString value_;
QString id_;
int result_{};
QString end_;
int base_{10};
bool allow_trailing_data_{false};
int base_{10};
};

class OptionDouble : public Option
{
public:
/* Special Member Functions */
OptionDouble() = default;
explicit OptionDouble(bool allow_trailing_data) :
allow_trailing_data_(allow_trailing_data)
{}

explicit(false) operator const QString& () const
{
Expand All @@ -187,10 +196,12 @@ class OptionDouble : public Option
return value_;
}

void init(const QString& id, bool allow_trailing_data, int /* base */) override;
void init(const QString& id) override;
void reset() override;
void set(const QString& s) override;
bool isValid(const QString& s) const;
double get_result(QString* end = nullptr) const;
bool trailing_data_allowed() const;

private:
QString value_;
Expand Down
4 changes: 2 additions & 2 deletions ozi.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class OziFormat : public Format
OptionString wptbgcolor;
OptionBool pack_opt;
int datum{};
OptionDouble proximityarg;
OptionDouble proximityarg{true};
double proximity{};
OptionString altunit_opt;
OptionString proxunit_opt;
Expand Down Expand Up @@ -175,7 +175,7 @@ class OziFormat : public Format
},
{
"proximity", &proximityarg, "Proximity distance",
"0", ARGTYPE_ALLOW_TRAILING_DATA | ARGTYPE_STRING, ARG_NOMINMAX, nullptr
"0", ARGTYPE_STRING, ARG_NOMINMAX, nullptr
},
{
"altunit", &altunit_opt, "Unit used in altitude values",
Expand Down
Loading

0 comments on commit 21603ae

Please sign in to comment.