More TODO's on the code:
important thing: All the .c files should compile and link against a test
app, this isn't happening right now because some of the C stuff depends on
GUI ( it was way worse on the gtk version, but it's still bad. )
On Wed, Apr 16, 2014 at 10:57 PM, Tomaz Canabrava <tcanabr...@kde.org>wrote:
> Dirk,
>
> Read this patch carefully, if I got anything wrong here I can crash the
> app, or worse, make your cat pregnant.
>
> This is an important patch regarding to the 'unittest' feature: it starts
> to decouple the app from the tangled web that it is today.
>
> @all: I added a lot of TODO's on the code, I'll work on them in due time (
> and by due time I mean "help me finish that" )
>
> Thanks,
>
> Tomaz
>
From 49b7e5842c19ce79725edd3c98b55dc7e0181741 Mon Sep 17 00:00:00 2001
From: Tomaz Canabrava <tomaz.canabr...@intel.com>
Date: Wed, 16 Apr 2014 23:52:52 -0300
Subject: [PATCH 2/2] Mark a lot of TODO's where I think it should be moved to
C code.
This marks a lot of todo's where I think there's core stuff
being mangled on the interface - we should remove this from
the interface to make testing and maintenability easier.
Signed-off-by: Tomaz Canabrava <tomaz.canabr...@intel.com>
---
dive.h | 2 ++
qt-ui/divelistview.cpp | 35 +++++++++++++++++++++++++----------
qt-ui/diveplanner.cpp | 4 +++-
qt-ui/mainwindow.cpp | 1 +
qt-ui/models.cpp | 7 ++++++-
qt-ui/printlayout.cpp | 1 +
qt-ui/simplewidgets.cpp | 1 +
qt-ui/subsurfacewebservices.cpp | 7 +++++--
8 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/dive.h b/dive.h
index 28cca58..c658083 100644
--- a/dive.h
+++ b/dive.h
@@ -846,7 +846,9 @@ struct diveplan {
struct divedatapoint *plan_add_segment(struct diveplan *diveplan, int duration, int depth, int o2, int he, int po2, bool entered);
void get_gas_string(int o2, int he, char *buf, int len);
struct divedatapoint *create_dp(int time_incr, int depth, int o2, int he, int po2);
+#if DEBUG_PLAN
void dump_plan(struct diveplan *diveplan);
+#endif
void plan(struct diveplan *diveplan, char **cached_datap, struct dive **divep, bool add_deco);
void delete_single_dive(int idx);
diff --git a/qt-ui/divelistview.cpp b/qt-ui/divelistview.cpp
index 493957f..86aca4e 100644
--- a/qt-ui/divelistview.cpp
+++ b/qt-ui/divelistview.cpp
@@ -242,6 +242,7 @@ void DiveListView::selectDives(const QList<int> &newDiveSelection)
QItemSelection newDeselected = selectionModel()->selection();
QModelIndexList diveList;
+ //TODO: This should be called find_first_selected_dive and be ported to C code.
int firstSelectedDive = -1;
/* context for temp. variables. */ {
int i = 0;
@@ -250,6 +251,7 @@ void DiveListView::selectDives(const QList<int> &newDiveSelection)
dive->selected = newDiveSelection.contains(i) == true;
if (firstSelectedDive == -1 && dive->selected) {
firstSelectedDive = i;
+ break;
}
}
}
@@ -444,6 +446,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS
const QAbstractItemModel *model = index.model();
struct dive *dive = (struct dive *)model->data(index, DiveTripModel::DIVE_ROLE).value<void *>();
if (!dive) { // it's a trip!
+ //TODO: deselect_trip_dives on c-code?
if (model->rowCount(index)) {
struct dive *child = (struct dive *)model->data(index.child(0, 0), DiveTripModel::DIVE_ROLE).value<void *>();
while (child) {
@@ -462,6 +465,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS
const QAbstractItemModel *model = index.model();
struct dive *dive = (struct dive *)model->data(index, DiveTripModel::DIVE_ROLE).value<void *>();
if (!dive) { // it's a trip!
+ //TODO: select_trip_dives on C code?
if (model->rowCount(index)) {
QItemSelection selection;
struct dive *child = (struct dive *)model->data(index.child(0, 0), DiveTripModel::DIVE_ROLE).value<void *>();
@@ -499,7 +503,8 @@ void DiveListView::merge_trip(const QModelIndex &a, int offset)
dive_trip_t *trip_a = (dive_trip_t *)a.data(DiveTripModel::TRIP_ROLE).value<void *>();
dive_trip_t *trip_b = (dive_trip_t *)b.data(DiveTripModel::TRIP_ROLE).value<void *>();
-
+ // TODO: merge_trip on the C code? some part of this needs to stay ( getting the trips from the model,
+ // but not the algorithm.
if (trip_a == trip_b || !trip_a || !trip_b)
return;
@@ -514,6 +519,7 @@ void DiveListView::merge_trip(const QModelIndex &a, int offset)
fixMessyQtModelBehaviour();
restoreSelection();
mark_divelist_changed(true);
+ //TODO: emit a signal to signalize that the divelist changed?
}
void DiveListView::mergeTripAbove()
@@ -528,6 +534,7 @@ void DiveListView::mergeTripBelow()
void DiveListView::removeFromTrip()
{
+ //TODO: move this to C-code.
int i;
struct dive *d;
for_each_dive(i, d) {
@@ -543,11 +550,12 @@ void DiveListView::removeFromTrip()
void DiveListView::newTripAbove()
{
- dive_trip_t *trip;
- int idx;
struct dive *d = (struct dive *)contextMenuIndex.data(DiveTripModel::DIVE_ROLE).value<void *>();
if (!d) // shouldn't happen as we only are setting up this action if this is a dive
return;
+ //TODO: port to c-code.
+ dive_trip_t *trip;
+ int idx;
rememberSelection();
trip = create_and_hookup_trip_from_dive(d);
for_each_dive(idx, d) {
@@ -573,15 +581,17 @@ void DiveListView::addToTripAbove()
void DiveListView::addToTrip(bool below)
{
- int idx, delta = (currentOrder == Qt::AscendingOrder) ? -1 : +1;
+ int delta = (currentOrder == Qt::AscendingOrder) ? -1 : +1;
+ struct dive *d = (struct dive *)contextMenuIndex.data(DiveTripModel::DIVE_ROLE).value<void *>();
+ rememberSelection();
+
+ //TODO: This part should be moved to C-code.
+ int idx;
dive_trip_t *trip = NULL;
struct dive *pd = NULL;
- struct dive *d = (struct dive *)contextMenuIndex.data(DiveTripModel::DIVE_ROLE).value<void *>();
- if (!d) // shouldn't happen as we only are setting up this action if this is a dive
- return;
if (below) // Should we add to the trip below instead?
delta *= -1;
- rememberSelection();
+
if (d->selected) { // we are right-clicking on one of possibly many selected dive(s)
// find the top selected dive, depending on the list order
if (delta == 1) {
@@ -613,6 +623,7 @@ void DiveListView::addToTrip(bool below)
}
trip->expanded = 1;
mark_divelist_changed(true);
+ //This part stays at C++ code.
reload(currentLayout, false);
restoreSelection();
fixMessyQtModelBehaviour();
@@ -627,6 +638,7 @@ void DiveListView::markDiveInvalid()
for_each_dive(i, d) {
if (!d->selected)
continue;
+ //TODO: this should be done in the future
// now mark the dive invalid... how do we do THAT?
// d->invalid = true;
}
@@ -643,10 +655,12 @@ void DiveListView::markDiveInvalid()
void DiveListView::deleteDive()
{
- int i;
struct dive *d = (struct dive *)contextMenuIndex.data(DiveTripModel::DIVE_ROLE).value<void *>();
if (!d)
return;
+
+ //TODO: port this to C-code.
+ int i;
// after a dive is deleted the ones following it move forward in the dive_table
// so instead of using the for_each_dive macro I'm using an explicit for loop
// to make this easier to understand
@@ -801,6 +815,7 @@ void DiveListView::loadImages()
for (int i = 0; i < fileNames.size(); ++i) {
if (readfile(fileNames.at(i).toUtf8().data(), &mem) <= 0)
continue;
+ //TODO: This inner code should be ported to C-Code.
retval = exif.parseFrom((const unsigned char *)mem.buffer, (unsigned)mem.size);
free(mem.buffer);
if (retval != PARSE_EXIF_SUCCESS)
@@ -808,7 +823,7 @@ void DiveListView::loadImages()
imagetime = shiftDialog.epochFromExiv(&exif);
if (!imagetime)
continue;
- imagetime += shiftDialog.amount();
+ imagetime += shiftDialog.amount(); // TODO: this should be cached and passed to the C-function
int j = 0;
struct dive *dive;
for_each_dive(j, dive) {
diff --git a/qt-ui/diveplanner.cpp b/qt-ui/diveplanner.cpp
index 1df0b34..f4325b4 100644
--- a/qt-ui/diveplanner.cpp
+++ b/qt-ui/diveplanner.cpp
@@ -599,6 +599,7 @@ void DivePlannerGraphics::drawProfile()
plannerModel->deleteTemporaryPlan();
return;
}
+ //TODO: divedatapoint_list_get_max_depth on C - code?
while (dp->next) {
if (dp->depth > max_depth)
max_depth = dp->depth;
@@ -1544,7 +1545,7 @@ void DivePlannerPointsModel::createTemporaryPlan()
// We just start with a surface node at time = 0
if (!stagingDive)
return;
-
+ //TODO: this thingy looks like it could be a good C-based function
diveplan.dp = NULL;
int lastIndex = -1;
for (int i = 0; i < rowCount(); i++) {
@@ -1614,6 +1615,7 @@ void DivePlannerPointsModel::createPlan()
return cancelPlan();
createTemporaryPlan();
+ //TODO: C-based function here?
plan(&diveplan, &cache, &tempDive, isPlanner());
copy_cylinders(stagingDive, tempDive);
int mean[MAX_CYLINDERS], duration[MAX_CYLINDERS];
diff --git a/qt-ui/mainwindow.cpp b/qt-ui/mainwindow.cpp
index 7b7abb8..e5287b4 100644
--- a/qt-ui/mainwindow.cpp
+++ b/qt-ui/mainwindow.cpp
@@ -386,6 +386,7 @@ void MainWindow::on_actionAddDive_triggered()
DivePlannerPointsModel::instance()->setPlanMode(DivePlannerPointsModel::ADD);
// now cheat - create one dive that we use to store the info tab data in
+ //TODO: C-function create_temporary_dive ?
struct dive *dive = alloc_dive();
dive->when = QDateTime::currentMSecsSinceEpoch() / 1000L + gettimezoneoffset();
dive->dc.model = "manually added dive"; // don't translate! this is stored in the XML file
diff --git a/qt-ui/models.cpp b/qt-ui/models.cpp
index a9b4c8f..479350a 100644
--- a/qt-ui/models.cpp
+++ b/qt-ui/models.cpp
@@ -444,7 +444,7 @@ void WeightModel::passInData(const QModelIndex &index, const QVariant &value)
}
}
}
-
+//TODO: Move to C
weight_t string_to_weight(const char *str)
{
const char *end;
@@ -469,6 +469,7 @@ lbs:
return weight;
}
+//TODO: Move to C.
depth_t string_to_depth(const char *str)
{
const char *end;
@@ -492,6 +493,7 @@ ft:
return depth;
}
+//TODO: Move to C.
pressure_t string_to_pressure(const char *str)
{
const char *end;
@@ -515,6 +517,7 @@ psi:
return pressure;
}
+//TODO: Move to C.
/* Imperial cylinder volumes need working pressure to be meaningful */
volume_t string_to_volume(const char *str, pressure_t workp)
{
@@ -547,6 +550,7 @@ l:
return volume;
}
+//TODO: Move to C.
fraction_t string_to_fraction(const char *str)
{
const char *end;
@@ -564,6 +568,7 @@ bool WeightModel::setData(const QModelIndex &index, const QVariant &value, int r
switch (index.column()) {
case TYPE:
if (!value.isNull()) {
+ //TODO: C-function weigth_system_set_description ?
if (!ws->description || tr(ws->description) != vString) {
// loop over translations to see if one matches
int i = -1;
diff --git a/qt-ui/printlayout.cpp b/qt-ui/printlayout.cpp
index e7e4fed..b583f6b 100644
--- a/qt-ui/printlayout.cpp
+++ b/qt-ui/printlayout.cpp
@@ -93,6 +93,7 @@ void PrintLayout::setup()
}
// go trought the dive table and find how many dives we are a going to print
+// TODO: C function: 'count_selected_dives' or something
int PrintLayout::estimateTotalDives() const
{
int total = 0, i = 0;
diff --git a/qt-ui/simplewidgets.cpp b/qt-ui/simplewidgets.cpp
index d8cd5d7..028e99e 100644
--- a/qt-ui/simplewidgets.cpp
+++ b/qt-ui/simplewidgets.cpp
@@ -215,6 +215,7 @@ void ShiftImageTimesDialog::syncCameraClicked()
connect(ui.dcTime, SIGNAL(dateTimeChanged(const QDateTime &)), this, SLOT(dcDateTimeChanged(const QDateTime &)));
}
+//TODO: This should be moved to C-Code.
time_t ShiftImageTimesDialog::epochFromExiv(EXIFInfo *exif)
{
struct tm tm;
diff --git a/qt-ui/subsurfacewebservices.cpp b/qt-ui/subsurfacewebservices.cpp
index d757928..feb98ba 100644
--- a/qt-ui/subsurfacewebservices.cpp
+++ b/qt-ui/subsurfacewebservices.cpp
@@ -40,7 +40,7 @@ static bool is_automatic_fix(struct dive *gpsfix)
}
#define SAME_GROUP 6 * 3600 // six hours
-
+//TODO: C Code. static functions are not good if we plan to have a test for them.
static bool merge_locations_into_dives(void)
{
int i, nr = 0, changed = 0;
@@ -94,7 +94,7 @@ static bool merge_locations_into_dives(void)
}
return changed > 0;
}
-
+//TODO: C-code.
static void clear_table(struct dive_table *table)
{
int i;
@@ -103,6 +103,7 @@ static void clear_table(struct dive_table *table)
table->nr = 0;
}
+// TODO: This looks like should be ported to C code. or a big part of it.
bool DivelogsDeWebServices::prepare_dives_for_divelogs(const QString &tempfile, const bool selected)
{
static const char errPrefix[] = "divelog.de-upload:";
@@ -455,6 +456,7 @@ void SubsurfaceWebServices::setStatusText(int status)
ui.status->setText(text);
}
+//TODO: C-Code.
/* requires that there is a <download> or <error> tag under the <root> tag */
void SubsurfaceWebServices::download_dialog_traverse_xml(xmlNodePtr node, unsigned int *download_status)
{
@@ -471,6 +473,7 @@ void SubsurfaceWebServices::download_dialog_traverse_xml(xmlNodePtr node, unsign
}
}
+// TODO: C-Code
unsigned int SubsurfaceWebServices::download_dialog_parse_response(const QByteArray &xml)
{
xmlNodePtr root;
--
1.9.2
_______________________________________________
subsurface mailing list
subsurface@hohndel.org
http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
