How can i clean my messy cod

suslucoder (30)
Hi everyone. Im a newbie, because of that i apolige. I have a messy code. I want to clean it and want it shorter. How can i do it?




if(item->parent() == gps)
{
if(scln == "A")
{
seri[0]->setColor(Qt::blue);
seri[0]->setName(gpsList.at(0));
chart->addSeries(seri2[0]);
seri[0]->attachAxis(axisX);
seri[0]->attachAxis(axisY);
chart->legend()->setVisible(true);
queuee[0].enqueue(0.25);

}

if(scln == "B")
{
seri[1]->setName(gpsList.at(1));
seri[1]->setColor(Qt::green);
chart->addSeries(seri2[1]);
seri[1]->attachAxis(axisX);
seri[1]->attachAxis(axisY);
chart->legend()->setVisible(true);
queuee[1].enqueue(0.96);
}

if(scln == "C")
{
seri[2]->setName(gpsList.at(2));
seri[2]->setColor(Qt::yellow);
chart->addSeries(seri[2]);
seri[2]->attachAxis(axisX);
seri[2]->attachAxis(axisY);
chart->legend()->setVisible(true);
queuee[2].enqueue(0.39);
}
if(scln == "D")
{
seri[3]->setName(gpsList.at(3));
seri[3]->setColor(Qt::darkBlue);
chart->addSeries(seri[3]);
seri[3]->attachAxis(axisX);
seri[3]->attachAxis(axisY);
chart->legend()->setVisible(true);
queuee[3].enqueue(0.66);
}



Last edited on
keskiverto (10402)
It is hard to tell what you actually are doing, but parametrization (and lookups) could reduce bloat:
1
2
3
4
5
6
7
8
9
10
11
 
auto colour[] { Qt::blue, Qt::green, Qt::yellow, Qt::darkBlue };
double queue[] { 0.25, 0.96, 0.39, 0.66 };

int x = ...
seri[x]->setName( gpsList.at(x) );
seri[x]->setColor( colour[x] );
chart->addSeries( seri[x] );
seri[x]->attachAxis( axisX );
seri[x]->attachAxis( axisY );
chart->legend()->setVisible( true );
queuee[x].enqueue( queue[x] );
Ganado (6811)
First, identify where you have repetition. Then, factor out the parts that are repeating, and supply the differences as parameters.

In each of your if statements, your instructions are of the following form:
1
2
3
4
5
6
7
 
seri[i]->setColor( ___ );
seri[i]->setName(gpsList.at(i));
chart->addSeries(seri2[i]);
seri[i]->attachAxis(axisX);
seri[i]->attachAxis(axisY);
chart->legend()->setVisible(true);
queuee[i].enqueue( ___ );


This can be factored into a function with three parameters: The index, the color, and the floating-point number enqueued. Note: Please use a better variable name than 'number'. I am only using 'number' because I don't know what it actually represents.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
 
void MyClass::update_gps_display(int i, Color color, double number)
{
    seri[i]->setColor(color);
    seri[i]->setName(gpsList.at(i));
    chart->addSeries(seri2[i]);
    seri[i]->attachAxis(axisX);
    seri[i]->attachAxis(axisY);
    chart->legend()->setVisible(true);
    queuee[i].enqueue(number);
}

void MyClass::Something()
{
    // ...
    
    if (scln == "A")
    {
        update_gps_display(0, Qt::blue, 0.25);
    }
    else if (scln == "B")
    {
        update_gps_display(1, Qt::green, 0.96);    
    }
    else if (scln == "C")
    {
        update_gps_display(2, Qt::yellow, 0.39);
    }
    else if (scln == "D")
    {
        update_gps_display(3, Qt::darkBlue, 0.66);
    }
}


Note: This assumes the other variables are available at a broader scope, such as the class scope. if they are not, I would suggest factoring these variables into some sort of struct or class.

Edit: I like keskiverto's idea even more. It allows you to leave everything in the current scope. You just need some logic to avoid the process if (x < 0 || x > 3).
Last edited on
suslucoder (30)
thank you for all your answers.
I will pick up which is more make sense to me, implement and i will update my reply
JRManx (603)
Well, if you need to keep your original code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
 
if (item->parent () == gps)
{
	if (scln == "A")
	{
		seri[0]->setColor (Qt::blue);
		seri[0]->setName (gpsList.at(0));
		
		chart->addSeries (seri2[0]);
		
		seri[0]->attachAxis (axisX);
		seri[0]->attachAxis (axisY);
		
		chart->legend()->setVisible (true);
		
		queuee[0].enqueue (0.25);
	}
	
	// Use else if's here instead of if's.  
	// It won't give you weird and nearly undetectable logic errors that way.
	else if (scln == "B")
	{
		seri[1]->setName (gpsList.at(1));
		seri[1]->setColor (Qt::green);
		
		chart->addSeries (seri2[1]);
		
		seri[1]->attachAxis (axisX);
		seri[1]->attachAxis (axisY);
		
		chart->legend()->setVisible (true);
		
		queuee[1].enqueue (0.96);
	}
	else if (scln == "C")
	{
		seri[2]->setName (gpsList.at(2));
		seri[2]->setColor (Qt::yellow);
		
		chart->addSeries (seri[2]);
		
		seri[2]->attachAxis (axisX);
		seri[2]->attachAxis (axisY);
		
		chart->legend()->setVisible (true);
		
		queuee[2].enqueue (0.39);
	}
	else if (scln == "D")
	{
		seri[3]->setName (gpsList.at(3));
		seri[3]->setColor (Qt::darkBlue);
		
		chart->addSeries (seri[3]);
		
		seri[3]->attachAxis (axisX);
		seri[3]->attachAxis (axisY);
		
		chart->legend()->setVisible (true);
		
		queuee[3].enqueue (0.66);
	}
	// Add an else in here for error checking
	else
	{
		std::cout << "Error. SCLN must be A, B, C, or D. \n";
	}
}

And, please don't double-post.
https://www.cplusplus.com/forum/beginner/277204/#msg1196468
It fills up the website with unnecessary junk that we have to wade through, and it causes people to think you may be a spammer.

Best,
max
againtry (2313)
It's a bit of a guess what this code snippet is supposed to do but it looks to me like a Qt charts application with multiple series being plotted, with the ability to filter in or out selected series.

If it is that then the legend marker capability of Qt is worth looking at and shows how GUI-based functionality is used:
https://doc.qt.io/qt-5/qtcharts-legendmarkers-example.html

It works using the same principle of reducing repetition that @keskiverto describes.

An array of QColors is OK but can also be left to a one-liner usage of QChart::ChartTheme's in the program.

doug4 (1538)
How can i clean my messy cod


I'm thinking something along the lines of a fishing knife and a gut bucket.

Afterwards, look for a nice tartar sauce recipe.
againtry (2313)
a nice tartar sauce recipe

Is that the sauce made out of hardened dental plaque or the one made out of half a Mongol's codpiece?
doug4 (1538)
Obviously, it must be the second, because it has the word "cod" right in it.
againtry (2313)
My uncertainty is based on both having the word 'tartar' in the name. They are both in my estimation equally unpalateable unless you find tartare sauce a worse item to spread on your cods, or cod if you only have one to satisfy your appetite.
JRManx (603)
"Tartare" or "tartar"? I don't think I'd like tartare sauce, mainly because I can't stomach raw meat.
Ganado (6811)
I blame my tartar on the Tartar tartar sauce I had with my steak tartare.
seeplus (6592)
Any spare chips to go with the cod?
JRManx (603)
I'll see if I can find some in my refrigerator.
deleted account xyzzy (5768)
How can i clean my messy cod

We've already done this party once before....

http://www.cplusplus.com/forum/beginner/265417/#msg1143056

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
#include <iostream>
#include <cmath>

double fish(int x)
{
   double xx = (x - 16) * (x - 16);
   return abs(x < 16 ? sqrt(64.0 - xx / 4.0) : 8 - 0.01 * xx);
}

int main()
{
   for (int j { 8 }; j >= -8; j--)
   {
      for (int i { }; i < 56; i++) std::cout << (abs(j) <= fish(i) ? '*' : ' ');
      std::cout << '\n';
   }
}
                *
         ******************                            *
      *************************                       **
    ******************************                   ***
   **********************************              *****
  *************************************           ******
 ****************************************       ********
 ******************************************   **********
********************************************************
 ******************************************   **********
 ****************************************       ********
  *************************************           ******
   **********************************              *****
    ******************************                   ***
      *************************                       **
         ******************                            *
                *
JRManx (603)
What does Codus Asteriskus taste like with Mongol tartar sauce made from tooth plaque...
againtry (2313)
Don't know but sounds like eating out of a gut bucket left in the sun for a whole day, even though I haven't tried that either.
doug4 (1538)
Thanks guys. I haven't been able to check the thread since my last post (2 days ago), and this made my day. I laughed harder than I expected to.

What a great way to start the morning.
againtry (2313)
What a great way to start the morning.

So what's for breakfast?
JRManx (603)
Messy cod tartare with Mongolian tartar sauce made from tooth plaque.

Just a thought, I wonder what happened to the OP. He's probably ROFL, reading this.

And, looking at his first post, he wants his cod shorter too. Cut off the head and tail...?
          *
   ******************                    
*************************                
****************************             
*******************************          
*********************************        
***********************************      
*************************************   *
*****************************************
*************************************   *
***********************************      
*********************************        
*******************************          
****************************             
*************************                
   ******************                    
          *
Topic archived. No new replies allowed.