5
\$\begingroup\$

I have performed this test and they have not given me a conclusive answer (they really have not given me any response). I have to paint a graphic, do you think the implementation is correct? Is there any possible improvement?


Given a GPS track with variable number of points of latitude, longitude and altitude (m). Example: 39.98069380,0.02221786,512.0

Assuming

  • You have Paint functions (you can assume that it paint point, line, a matrix of pixels or other kind of functions)
  • You have list of points in memory in a matrix named Trackpoints[latitude, longitude,altitude]
  • The screen resolution is 128x128
  • Track can have 2 to 5000 points (consider performance issues)
  • Distance between points should be determined by coordinates

Task

  • Create a working module to paint graph of distance-altitude (meters)
  • Graph should expand to whole X axis.
  • Grey Horizontal bands are optional
  • Vertical scale values are optional
  • Fill graph area is optional
  • Graph example attached enter image description here

Now my code:

I used qCustomPlot library and tryed to do the code as simple as possible just with 4 classes

Map.h

#ifndef MAP_H
#define MAP_H

#include <vector>
#include <tuple>
#include <random>

struct coordinate{
    double longitude, altitude, latitude;
};

class Map
{
public:
    Map(int total_points, double top_point);
    int total_coordinates;
    double highest_point;
    std::vector<coordinate> matrix_points;

private:
    double RandomNumber();
    std::vector<coordinate> GenerateCoordinates();
};
#endif // MAP_H

Map.cpp

#include "map.h"

using namespace std;

Map::Map(int total_points, double top_point)
{
    total_coordinates = total_points;
    highest_point = top_point;

   matrix_points = GenerateCoordinates();
}

double Map::RandomNumber()
{
    double random_value;

    // Generate a random between a margin
    static random_device rd;
    default_random_engine generator(rd());
    uniform_real_distribution<double> distribution(0, highest_point);

    random_value = distribution(generator);

    return random_value;
}

vector<coordinate> Map::GenerateCoordinates()
{
    coordinate coordi;
    vector<coordinate> track_points;

    for(int i = 0; i < total_coordinates; i++)
    {
        coordi.longitude = i;
        coordi.altitude = RandomNumber();
        coordi.latitude = RandomNumber();

        track_points.push_back(coordi);
    }

    return track_points;
}

MainWindow.cpp

#include "mainwindow.h"
#include "ui_mainwindow.h"
#include "map.h"

using namespace std;

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);

    Map map(50, 50.6);

    coordinate coordi;
    coordi.longitude = 39.98069380;
    coordi.altitude = 0.02221786;
    coordi.latitude = 512.0;

    Paint();
    Paint(map);
    Paint(coordi);

    ui->customPlot->replot();
}

// Paint map
void MainWindow::Paint()
{
    // Create graph and assign data to it:
    ui->customPlot->addGraph();

    // First graph will be filled with translucent blue
    ui->customPlot->graph(0)->setBrush(QBrush(QColor(0, 0, 255, 20)));

    // Let the ranges scale themselves so graph 0 fits perfectly in the visible area:
    ui->customPlot->graph(0)->rescaleAxes();

    // Set blank axis lines
    ui->customPlot->xAxis->setTicks(false);
    ui->customPlot->xAxis->setTickLabels(false);
    ui->customPlot->yAxis->setTicks(false);
    ui->customPlot->yAxis->setTickLabels(false);

    // make top right axes clones of bottom left axes:
    ui->customPlot->axisRect()->setupFullAxesBox();
}

void MainWindow::Paint(const Map map)
{
    QVector<double> x(map.total_coordinates), y(map.total_coordinates);

    for(int i = 0; i < map.total_coordinates; i++)
    {
        x[i] = map.matrix_points[i].longitude;
        y[i] = map.matrix_points[i].altitude;
    }

    // Create graph and assign data to it:
    ui->customPlot->addGraph();
    ui->customPlot->graph(0)->setData(x, y);

    // Set axes ranges, so we see all data:
    ui->customPlot->xAxis->setRange(0, x.last());
    ui->customPlot->yAxis->setRange(0, map.highest_point);
}

void MainWindow::Paint(const coordinate coordi)
{
    QCPAxis* yAxis = ui->customPlot->axisRect(0)->axis(QCPAxis::atLeft);

    // Red line
    QCPItemLine *item = new QCPItemLine(ui->customPlot);
    item->setPen(QPen(Qt::red));
    item->start->setCoords(coordi.longitude, 0); // assuming 0 is always the lowest number
    item->end->setCoords(coordi.longitude, yAxis->pixelToCoord(ui->customPlot->size().height()));
}

MainWindow::~MainWindow()
{
    delete ui;
}

MainWindow.h

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include "map.h"

namespace Ui {
class MainWindow;
}

class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();

private:
    Ui::MainWindow *ui;

    void Paint();
    void Paint(const Map map);
    void Paint(const coordinate coordi);

};

#endif // MAINWINDOW_H
\$\endgroup\$
  • \$\begingroup\$ You used the comparative-review tag but I don't see multiple solutions there. Could you clarify this in your quesiton or remove the tag so that it is not confusing people? And could you explain what you mean by Test in the title? There are no unit-tests. What kind of test is it? \$\endgroup\$ – t3chb0t Jun 9 '18 at 12:43
  • \$\begingroup\$ @t3chb0t I think this is an interview-question rather than a programming-challenge and OP simply used the wrong words/tags to describe it. \$\endgroup\$ – yuri Jun 9 '18 at 12:47
  • \$\begingroup\$ Doesn't using existing plotting library defeat the purpose of a challenge? \$\endgroup\$ – Nikita B Jun 9 '18 at 12:48
  • 1
    \$\begingroup\$ Depends on the challenge. “I know a library function to do that” is the right answer in most real-world “challenges” that I don’t see often enough. \$\endgroup\$ – JDługosz Jun 9 '18 at 22:26
  • \$\begingroup\$ @t3chb0t True it isn't a comparative, sorry for the msitake, it's my first time asking un SE. Yes it is not a test at all, it's like a programming challenge, sorry once. \$\endgroup\$ – felixjimcal Jun 10 '18 at 14:15
3
\$\begingroup\$

Don’t write using namespace std;.

You can, however, in a CPP file (not H file) or inside a function put individual using std::string; etc. (See SF.7.)


coordinate coordi;
coordi.longitude = 39.98069380;
coordi.altitude = 0.02221786;
coordi.latitude = 512.0;

Use an initializer, not a list of assignments after default initialization. In this case, it is simply an aggregate, so you can write

coordinate coordi {39.98069380, 0.02221786, 512.0};

without having to change anything about the class.


The style in C++ is to put the * or & with the type, not the identifier. This is called out specifically near the beginning of Stroustrup’s first book, and is an intentional difference from C style.


I see a couple places where you are using ye olde syntax for intializations.


MainWindow::~MainWindow()
{
    delete ui;
}

⧺C.149 — no naked new or delete.

You should probably make this a unique_ptr as a drop-in replacement without otherwise changing the architecture. Then, you can get rid of the manually written destructor completely.

Or, in this case I see the object has the same lifetime as the container and you don’t do any pointer manipulation or whatever. So why do you need a pointer at all, instead of just containing the object by value?

\$\endgroup\$
  • \$\begingroup\$ Totally agree with using namespace std argument, also with the object initialization, not using a list. About the "delete ui" it's autogenerated code. \$\endgroup\$ – felixjimcal Jun 10 '18 at 14:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.