Skip to content

Commit f2c5310

Browse files
chdinhchdinh
committed
refactor: code quality offensive — iterations 1 & 2
Iteration 1 — Highest impact fixes (21 files): - simplex_algorithm.h: eliminate void* callback → type-safe templates + lambdas - fiff_byte_swap.h: 14 C-style casts → reinterpret_cast - fiff_tag.h: C-style casts → reinterpret_cast, toFloat()/toDouble() → const return - fs_surface.cpp: 11 C-style casts → static_cast/reinterpret_cast - math/numerics.cpp: 3 C-style casts → static_cast - com/rt_client.cpp: printf → qInfo, 2 casts → static_cast - Const-correctness cascade across 10 files from toFloat/toDouble change Iteration 2 — Library printf elimination (20 files, ~380 calls): - ALL printf eliminated from src/libraries/ (0 remaining) - Each call categorized: qInfo (progress), qCritical (errors), qWarning (recoverable), qDebug (internal) - Major files: mne_inverse_operator(66), mne_source_space(53), mne_raw_data(39), mne_ctf_comp_data_set(31), and 16 more Also adds doc/dev-notes/optimization-requirements.md — living document for tracking code quality metrics, priority matrix, audit process, design pattern catalogue, and progress.
1 parent d4b9903 commit f2c5310

37 files changed

+1226
-448
lines changed

‎doc/dev-notes/optimization-requirements.md‎

Lines changed: 732 additions & 0 deletions
Large diffs are not rendered by default.

‎src/applications/mne_browse/Models/annotationmodel.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -820,15 +820,15 @@ bool AnnotationModel::loadAnnotationFif(QFile& qFile)
820820
}
821821

822822
if(directoryEntry->kind == FiffMneBaselineMin) {
823-
if(float* values = tag->toFloat()) {
823+
if(const float* values = tag->toFloat()) {
824824
const int count = tag->size() / static_cast<int>(sizeof(float));
825825
onsetSeconds.resize(count);
826826
for(int index = 0; index < count; ++index) {
827827
onsetSeconds[index] = values[index];
828828
}
829829
}
830830
} else if(directoryEntry->kind == FiffMneBaselineMax) {
831-
if(float* values = tag->toFloat()) {
831+
if(const float* values = tag->toFloat()) {
832832
const int count = tag->size() / static_cast<int>(sizeof(float));
833833
endSeconds.resize(count);
834834
for(int index = 0; index < count; ++index) {

‎src/libraries/com/rt_client/rt_client.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,15 +175,17 @@ void RtClient::run()
175175
if(kind == FIFF_DATA_BUFFER)
176176
{
177177
to += matData.cols();
178-
printf("Reading %d ... %d = %9.3f ... %9.3f secs...", from, to, ((float)from)/m_pFiffInfo->sfreq, ((float)to)/m_pFiffInfo->sfreq);
178+
qInfo("Reading %d ... %d = %9.3f ... %9.3f secs...", from, to,
179+
static_cast<float>(from)/m_pFiffInfo->sfreq,
180+
static_cast<float>(to)/m_pFiffInfo->sfreq);
179181
from += matData.cols();
180182

181183
emit rawBufferReceived(matData);
182184
}
183185
else if(FIFF_DATA_BUFFER == FIFF_BLOCK_END)
184186
m_bIsRunning = false;
185187

186-
printf("[done]\n");
188+
qInfo("[done]");
187189
}
188190

189191
//

‎src/libraries/com/rt_client/rt_data_client.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ FiffInfo::SPtr RtDataClient::readInfo()
110110
t_fiffStream.read_rt_tag(t_pTag);
111111
if(t_pTag->kind == FIFF_BLOCK_START && *(t_pTag->toInt()) == FIFFB_MEAS_INFO)
112112
{
113-
printf("FIFF_BLOCK_START FIFFB_MEAS_INFO\n");
113+
qInfo("FIFF_BLOCK_START FIFFB_MEAS_INFO");
114114
t_bReadMeasBlockStart = true;
115115
}
116116
}
@@ -349,7 +349,7 @@ FiffInfo::SPtr RtDataClient::readInfo()
349349
// END MEAS
350350
if(t_pTag->kind == FIFF_BLOCK_END && *t_pTag->toInt() == FIFFB_MEAS_INFO)
351351
{
352-
printf("FIFF_BLOCK_END FIFFB_MEAS_INFO\n");
352+
qInfo("FIFF_BLOCK_END FIFFB_MEAS_INFO");
353353
t_bReadMeasBlockEnd = true;
354354
}
355355
}
@@ -385,7 +385,7 @@ MetaData RtDataClient::readMetadata()
385385
t_fiffStream.read_rt_tag(t_pTag);
386386
if(t_pTag->kind == FIFF_BLOCK_START && *(t_pTag->toInt()) == FIFFB_MEAS_INFO)
387387
{
388-
printf("FIFF_BLOCK_START FIFFB_MEAS_INFO\n");
388+
qInfo("FIFF_BLOCK_START FIFFB_MEAS_INFO");
389389
t_bReadMeasBlockStart = true;
390390
}
391391
}
@@ -629,7 +629,7 @@ MetaData RtDataClient::readMetadata()
629629
// END MEAS
630630
if(t_pTag->kind == FIFF_BLOCK_END && *t_pTag->toInt() == FIFFB_MEAS_INFO)
631631
{
632-
printf("FIFF_BLOCK_END FIFFB_MEAS_INFO\n");
632+
qInfo("FIFF_BLOCK_END FIFFB_MEAS_INFO");
633633
t_bReadMeasBlockEnd = true;
634634
}
635635
}
@@ -670,7 +670,7 @@ void RtDataClient::readRawBuffer(qint32 p_nChannels,
670670
if(kind == FIFF_DATA_BUFFER)
671671
{
672672
qint32 nSamples = (t_pTag->size()/4)/p_nChannels;
673-
data = MatrixXf(Map< MatrixXf >(t_pTag->toFloat(), p_nChannels, nSamples));
673+
data = MatrixXf(Map<const MatrixXf>(t_pTag->toFloat(), p_nChannels, nSamples));
674674
}
675675
// else
676676
// data = tag.data;

‎src/libraries/com/rt_command/command.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ Command::Command( const QString &p_sCommand, const QString &p_sDescription,
137137
}
138138
else
139139
{
140-
printf("error: description vector hasn't the same size like parameter map.\n");
140+
qCritical("error: description vector hasn't the same size like parameter map.\n");
141141
return;
142142
}
143143
}

‎src/libraries/com/rt_command/command_parser.cpp‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include <QStringList>
5050
#include <QJsonObject>
5151
#include <QJsonDocument>
52+
#include <QDebug>
5253

5354
//=============================================================================================================
5455
// USED NAMESPACES
@@ -113,7 +114,7 @@ bool CommandParser::parse(const QString &p_sInput, QStringList &p_qListCommandsP
113114
for(it = t_jsonObjectCommand.begin(); it != t_jsonObjectCommand.end(); ++it)
114115
{
115116
//Print Command
116-
printf("%s\r\n", it.key().toUtf8().constData());
117+
qInfo("%s\r\n", it.key().toUtf8().constData());
117118

118119
if(exists(it.key()))
119120
{
@@ -127,7 +128,7 @@ bool CommandParser::parse(const QString &p_sInput, QStringList &p_qListCommandsP
127128
//append the parameters
128129
for(itParam= t_jsonObjectParameters.begin(); itParam != t_jsonObjectParameters.end(); ++itParam)
129130
{
130-
printf(" %s", itParam.value().toString().toUtf8().constData());
131+
qInfo(" %s", itParam.value().toString().toUtf8().constData());
131132
//ToDo do a cross check with the param naming and key
132133
m_rawCommand.pValues().append(itParam.value().toString());
133134
// qDebug() << itParam.key() << " + " << itParam.value().toString();
@@ -136,19 +137,19 @@ bool CommandParser::parse(const QString &p_sInput, QStringList &p_qListCommandsP
136137
//Notify attached command manager
137138
notify();
138139
}
139-
printf("\r\n");
140+
qInfo("\r\n");
140141
}
141142
}
142143
else
143144
{
144145
QStringList t_qCommandList = p_sInput.split(" ");
145146

146147
//Print command
147-
printf("%s\r\n", t_qCommandList[0].toUtf8().constData());
148+
qInfo("%s\r\n", t_qCommandList[0].toUtf8().constData());
148149

149150
if(!exists(t_qCommandList[0]))
150151
{
151-
printf("\r\n");
152+
qInfo("\r\n");
152153
return false;
153154
}
154155

@@ -163,11 +164,11 @@ bool CommandParser::parse(const QString &p_sInput, QStringList &p_qListCommandsP
163164
//Parse Parameters
164165
for(qint32 i = 1; i < t_qCommandList.size(); ++i)
165166
{
166-
printf(" %s", t_qCommandList[i].toUtf8().constData());
167+
qInfo(" %s", t_qCommandList[i].toUtf8().constData());
167168
m_rawCommand.pValues().append(t_qCommandList[i]);
168169
}
169170
}
170-
printf("\r\n");
171+
qInfo("\r\n");
171172
notify();
172173
}
173174

‎src/libraries/fiff/fiff_byte_swap.h‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ namespace FIFFLIB
5454
*/
5555
inline qint16 swap_short(qint16 source)
5656
{
57-
unsigned char *csource = (unsigned char *)(&source);
57+
auto *csource = reinterpret_cast<unsigned char *>(&source);
5858
qint16 result;
59-
unsigned char *cresult = (unsigned char *)(&result);
59+
auto *cresult = reinterpret_cast<unsigned char *>(&result);
6060
cresult[0] = csource[1];
6161
cresult[1] = csource[0];
6262
return result;
@@ -68,9 +68,9 @@ inline qint16 swap_short(qint16 source)
6868
*/
6969
inline qint32 swap_int(qint32 source)
7070
{
71-
unsigned char *csource = (unsigned char *)(&source);
71+
auto *csource = reinterpret_cast<unsigned char *>(&source);
7272
qint32 result;
73-
unsigned char *cresult = (unsigned char *)(&result);
73+
auto *cresult = reinterpret_cast<unsigned char *>(&result);
7474
cresult[0] = csource[3];
7575
cresult[1] = csource[2];
7676
cresult[2] = csource[1];
@@ -84,7 +84,7 @@ inline qint32 swap_int(qint32 source)
8484
*/
8585
inline void swap_intp(qint32 *source)
8686
{
87-
unsigned char *csource = (unsigned char *)(source);
87+
auto *csource = reinterpret_cast<unsigned char *>(source);
8888
unsigned char c;
8989
c = csource[3]; csource[3] = csource[0]; csource[0] = c;
9090
c = csource[2]; csource[2] = csource[1]; csource[1] = c;
@@ -96,9 +96,9 @@ inline void swap_intp(qint32 *source)
9696
*/
9797
inline qint64 swap_long(qint64 source)
9898
{
99-
unsigned char *csource = (unsigned char *)(&source);
99+
auto *csource = reinterpret_cast<unsigned char *>(&source);
100100
qint64 result;
101-
unsigned char *cresult = (unsigned char *)(&result);
101+
auto *cresult = reinterpret_cast<unsigned char *>(&result);
102102
cresult[0] = csource[7];
103103
cresult[1] = csource[6];
104104
cresult[2] = csource[5];
@@ -116,7 +116,7 @@ inline qint64 swap_long(qint64 source)
116116
*/
117117
inline void swap_longp(qint64 *source)
118118
{
119-
unsigned char *csource = (unsigned char *)(source);
119+
auto *csource = reinterpret_cast<unsigned char *>(source);
120120
unsigned char c;
121121
c = csource[0]; csource[0] = csource[7]; csource[7] = c;
122122
c = csource[1]; csource[1] = csource[6]; csource[6] = c;
@@ -130,9 +130,9 @@ inline void swap_longp(qint64 *source)
130130
*/
131131
inline float swap_float(float source)
132132
{
133-
unsigned char *csource = (unsigned char *)(&source);
133+
auto *csource = reinterpret_cast<unsigned char *>(&source);
134134
float result;
135-
unsigned char *cresult = (unsigned char *)(&result);
135+
auto *cresult = reinterpret_cast<unsigned char *>(&result);
136136
cresult[0] = csource[3];
137137
cresult[1] = csource[2];
138138
cresult[2] = csource[1];
@@ -146,7 +146,7 @@ inline float swap_float(float source)
146146
*/
147147
inline void swap_floatp(float *source)
148148
{
149-
unsigned char *csource = (unsigned char *)(source);
149+
auto *csource = reinterpret_cast<unsigned char *>(source);
150150
unsigned char c;
151151
c = csource[3]; csource[3] = csource[0]; csource[0] = c;
152152
c = csource[2]; csource[2] = csource[1]; csource[1] = c;
@@ -158,7 +158,7 @@ inline void swap_floatp(float *source)
158158
*/
159159
inline void swap_doublep(double *source)
160160
{
161-
unsigned char *csource = (unsigned char *)(source);
161+
auto *csource = reinterpret_cast<unsigned char *>(source);
162162
unsigned char c;
163163
c = csource[7]; csource[7] = csource[0]; csource[0] = c;
164164
c = csource[6]; csource[6] = csource[1]; csource[1] = c;

‎src/libraries/fiff/fiff_raw_data.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ bool FiffRawData::read_raw_segment(MatrixXd& data,
305305
else if(t_pTag->type == FIFFT_INT)
306306
one = cal*(Map< MatrixXi >( t_pTag->toInt(),nchan, thisRawDir.nsamp)).cast<double>();
307307
else if(t_pTag->type == FIFFT_FLOAT)
308-
one = cal*(Map< MatrixXf >( t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
308+
one = cal*(Map<const MatrixXf>(t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
309309
else if(t_pTag->type == FIFFT_SHORT)
310310
one = cal*(Map< MatrixShort >( t_pTag->toShort(),nchan, thisRawDir.nsamp)).cast<double>();
311311
else
@@ -332,7 +332,7 @@ bool FiffRawData::read_raw_segment(MatrixXd& data,
332332
}
333333
else if(t_pTag->type == FIFFT_FLOAT)
334334
{
335-
tmp_data = (Map< MatrixXf > ( t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
335+
tmp_data = (Map<const MatrixXf>(t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
336336

337337
for(r = 0; r < sel.size(); ++r)
338338
newData.block(r,0,1,thisRawDir.nsamp) = tmp_data.block(sel[r],0,1,thisRawDir.nsamp);
@@ -359,7 +359,7 @@ bool FiffRawData::read_raw_segment(MatrixXd& data,
359359
else if(t_pTag->type == FIFFT_INT)
360360
one = mult*(Map< MatrixXi >( t_pTag->toInt(),nchan, thisRawDir.nsamp)).cast<double>();
361361
else if(t_pTag->type == FIFFT_FLOAT)
362-
one = mult*(Map< MatrixXf >( t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
362+
one = mult*(Map<const MatrixXf>(t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
363363
else
364364
qWarning("Data Storage Format not known yet [3]!! Type: %d\n", t_pTag->type);
365365
}
@@ -639,7 +639,7 @@ bool FiffRawData::read_raw_segment(MatrixXd& data,
639639
else if(t_pTag->type == FIFFT_INT)
640640
one = cal*(Map< MatrixXi >( t_pTag->toInt(),nchan, thisRawDir.nsamp)).cast<double>();
641641
else if(t_pTag->type == FIFFT_FLOAT)
642-
one = cal*(Map< MatrixXf >( t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
642+
one = cal*(Map<const MatrixXf>(t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
643643
else if(t_pTag->type == FIFFT_SHORT)
644644
one = cal*(Map< MatrixShort >( t_pTag->toShort(),nchan, thisRawDir.nsamp)).cast<double>();
645645
else
@@ -667,7 +667,7 @@ bool FiffRawData::read_raw_segment(MatrixXd& data,
667667
}
668668
else if(t_pTag->type == FIFFT_FLOAT)
669669
{
670-
MatrixXd tmp_data = (Map< MatrixXf > ( t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
670+
MatrixXd tmp_data = (Map<const MatrixXf>(t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
671671

672672
for(r = 0; r < sel.size(); ++r)
673673
newData.block(r,0,1,thisRawDir.nsamp) = tmp_data.block(sel[r],0,1,thisRawDir.nsamp);
@@ -694,7 +694,7 @@ bool FiffRawData::read_raw_segment(MatrixXd& data,
694694
else if(t_pTag->type == FIFFT_INT)
695695
one = mult*(Map< MatrixXi >( t_pTag->toInt(),nchan, thisRawDir.nsamp)).cast<double>();
696696
else if(t_pTag->type == FIFFT_FLOAT)
697-
one = mult*(Map< MatrixXf >( t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
697+
one = mult*(Map<const MatrixXf>(t_pTag->toFloat(),nchan, thisRawDir.nsamp)).cast<double>();
698698
else
699699
qWarning("Data Storage Format not known yet [3]!! Type: %d\n", t_pTag->type);
700700
}

‎src/libraries/fiff/fiff_stream.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -624,11 +624,11 @@ bool FiffStream::read_cov(const FiffDirNode::SPtr& p_Node, fiff_int_t cov_kind,
624624
//
625625
if (tag->type == FIFFT_DOUBLE)
626626
{
627-
cov_diag = Map<VectorXd>(tag->toDouble(),dim);
627+
cov_diag = Map<const VectorXd>(tag->toDouble(),dim);
628628
}
629629
else if (tag->type == FIFFT_FLOAT)
630630
{
631-
cov_diag = Map<VectorXf>(tag->toFloat(),dim).cast<double>();
631+
cov_diag = Map<const VectorXf>(tag->toFloat(),dim).cast<double>();
632632
}
633633
else {
634634
qCritical("Illegal data type for covariance matrix\n");
@@ -645,11 +645,11 @@ bool FiffStream::read_cov(const FiffDirNode::SPtr& p_Node, fiff_int_t cov_kind,
645645
nn = dim*(dim+1)/2;
646646
if (tag->type == FIFFT_DOUBLE)
647647
{
648-
vals = Map<VectorXd>(tag->toDouble(),nn);
648+
vals = Map<const VectorXd>(tag->toDouble(),nn);
649649
}
650650
else if (tag->type == FIFFT_FLOAT)
651651
{
652-
vals = Map<VectorXf>(tag->toFloat(),nn).cast<double>();
652+
vals = Map<const VectorXf>(tag->toFloat(),nn).cast<double>();
653653
}
654654
else
655655
{
@@ -726,7 +726,7 @@ bool FiffStream::read_cov(const FiffDirNode::SPtr& p_Node, fiff_int_t cov_kind,
726726
FiffTag::UPtr tag2;
727727
if (current->find_tag(this, FIFF_MNE_COV_EIGENVALUES, tag1) && current->find_tag(this, FIFF_MNE_COV_EIGENVECTORS, tag2))
728728
{
729-
eig = VectorXd(Map<VectorXd>(tag1->toDouble(),dim));
729+
eig = VectorXd(Map<const VectorXd>(tag1->toDouble(),dim));
730730
eigvec = tag2->toFloatMatrix().cast<double>();
731731
eigvec.transposeInPlace();
732732
}

0 commit comments

Comments
 (0)