We’re rewarding the question askers & reputations are being recalculated! Read more.

Hot answers tagged

12

The typical MVC pattern has a data flow like this: |--->Model -> View -> Controller ----| | | |--------------------------------------------| So data flows from the Model to the View (When State is updated) So data flows from the View to the Controller (When the user interacts) So data flows ...


11

This will not be a full review of the code at all since I don't really review how it works, but here are some little things that may be of interest to make it better anyway :) Qt5 new connection syntax Qt5 introduces a new syntax for signals and slots which allows you to directly pass function pointers instead of relying on the Qt preprocessor to do the ...


11

In Qt, there is a concept of parents and a hierarchy. In essence, it means that when a qobject owns another qobject (the other qobject's parent is the first) then the parent will take care of cleaning its children. There are 2 ways an object becomes a child of another: when it is set in the constructor or when reparented. The qwidget family automatically ...


8

The code you have is unsafe, because pause() and cancelWork() are called from outside the worker's thread, and they use no locking when modifying the worker's members. In general, prefer to interact with objects in other threads by invoking their slots via queued connections, unless there's a pressing reason to act immediately. This means you don't need ...


7

That's definitely an interesting approach. However, I would say that this interface is trying to accomplish too much. That is, average should just be responsible for calculating the average value over some range [first, last) and not worry about any indirection. E.g., only provide the signature template< class InputIterator > double average( ...


7

It's not at all clear why you've declared/defined this: MyWorker::~MyWorker() { } Unless there's something specific that this accomplishes (which I don't see) you're better off without it. void MyWorker::restart() { this -> m_pauseRequired = false; this -> m_pauseManager.wakeAll(); } I know there are people here who disagree about this, but ...


7

Here are my thoughts: It's a simple class that allocates quite a few times on the heap. You could minimize the number of implicit allocations it does. The QTimer::singleShot creates a temporary QObject instance and allocates a bunch on the heap. You can avoid it by handling a single-shot timer explicitly using timerEvent. You also avoid the need to set up ...


7

I see a number of things that may help you improve your code. Use named constants There are a number of constant strings which are used repeatedly. If they're used more than once, it's probably a sign that you should use named constants instead. constexpr static char[] JPG_EXT{".jpg"}; constexpr static char[] PNG_EXT{".png"}; Consolidate tests in ...


7

Well, it's not much of a mystery. Your code is simple and pretty. Their code is ugly because it performs exactly what it needs to do, and no more: } else if (!strncmp(sz,"vertex",6) && ::IsSpaceOrNewLine(*(sz+6))) { // vertex 1.50000 1.50000 0.00000 if (faceVertexCounter >= 3) { ASSIMP_LOG_ERROR("STL: a facet with more than 3 ...


6

Is this interface complete? Does it attempt to do too much or too little? In my opinion, interfaces should be as small as possible, providing only methods that are really required to be called without knowing (or casting to) the actual subclass. I don't know how you use it, but my first impression is: your interface contains too many methods... Are there ...


6

Not being a frequent Qt user, I don't have much to say. But there a few things: SNIPPETS_FILE: That would be a lot better as a function. Macros and C++ are not a very good match. Macros have quite a few drawback, the most annoying ones are probably not respecting scope and being able to silently redefine them by mistake. I'm not saying that macros don't ...


6

First I would like to say that I'm not an expert on rfkill; so my comments are more general and may be slightly off for this specific case. Despite the missing error handling (which you mention) I see only two problems. Not primarily with your code, but with your assumptions: You are correct to avoid __attribute__((packed)), but the reasons are IMHO not ...


6

I see a few things that may help you improve your code. Pass by const reference where practical In every case that a QString ip is passed, the code could instead use a const QString &ip avoiding pointless multiple copies and also allowing the addition of a const IP address. Consider large files Reading the entire file into memory using QTextStream::...


6

It looks like you're manually trying to calculate the amount of hours since birth, which will be difficult because of leap years and the general difficulty of working with dates. I would recommend using the datetime module instead: from datetime import datetime current_time = datetime.now() # Use the information you collect in your GUI to enter a datetime ...


6

Cleaner and clearer it can certainly be: your function is called isFrameMatchedByFilter but returns true if there is no filter in the filter list: does that mean the frame is matched by a void filter? It's rather unclear, you need to either find another name or change the behavior of the function. your argument is called frameToFilter, but it's a bad idea. ...


5

I'd have serious doubts about its being the fastest possible code, but just for fun let's consider the simplest possible code to generate correct answers. The standard library has next_permutation, which can generate all the possible permutations of a collection. So, let's consider how we do this. We start with a 2D array of distances between cities. We ...


5

Some of these comments can be removed: Too obvious: // Do nothing if we don't have a selection. Noisy: // Done! These initialized variables: int spos = curs.anchor(), epos = curs.position(); should be on separate lines for better maintenance: int spos = curs.anchor(); int epos = curs.position(); No need to do a manual swap: if(spos > epos) { ...


5

Case for not using a class It's not clear to me whether you need a class for what you are doing. Unless you need to be able to construct an instance of ConfigurationReader and pass it around, I would say get rid of the class. You can have a function with the simple interface QHash<QString, QString> getConfigurationMap(QFile& input); I would ...


5

One thing that would help simplify your implementation is to fall back to already implemented code. For example, you could implement subtraction as addition using the negative of the second fraction. Likewise you could implement division as multiplication with the reciprocal of the second fraction. Another point about readability: because you are ...


5

Disclaimer: I'm not familiar with Qt With that in mind I still have some points I can mention. My worry is that this approach may be inefficient, given how many variables a State has and how long a user's input could potentially be. Granted, most of the State members are only bytes, but there are also two ints each. If this approach of using a ...


5

I see a number of things that may help you improve your program. Use proper case for file names We all know that Windows is not case sensitive with respect to file names, but pretty much every other operating system out there (Apple's OS X, Linux, BSD, etc.) is. For that reason, when you have a class named Generator it should be in a file named Generator....


5

Seems like you've made a lot of improvements since your previous post! Let's get into the review! 1. General Overview a. Use the on_<objectName>_<signal> Naming Scheme for Slots This naming scheme tells the moc to automatically connect a slot with the corresponding <signal> of <objectName> from the UI. We then don't need to call ...


5

Note that we don't have a fully-compilable program, or the example data. But you do! So you can actually measure what's taking up the time using a profiler. However, I would guess that the following lines are the problem: QString line = QString::fromLatin1(line_BA); // Split line by spaces QStringList words = line.split(QRegExp("\\s+"), QString:...


4

After some research I found that it's slightly more efficient to get the half quaternion directly from the double quaternion: QVector3D rotaxis = QVector3D::crossProduct (vec1, vec2); double cos = QVector3D::dotProduct (vec1, vec2); // rotaxis and cos describe the double quat // add 1 to cos so normalizing will 0.5 lerp between the null quat and the double ...


4

First of all: your code is not bad; I agree with Wayne Conrad. I have a minor remark about the connect statement. You are currently using Qt 4.8, so this is fine as it is. However with Qt 5 a new signal/slot syntax was introduced. Your question about threading: Don't be afraid of QThread, they're just threads too. The problem with QThread is only this: a ...


4

In general I like that you didn't try and force OO on something that works very well without it, but it would be simpler, cleaner if you were to just put all those functions in a namespace instead of a class (IIRC the metadata-generating macros for enums work outside classes, too). you can use global static functions or anonymous namespaces in your .cpp ...


4

A quick note: I haven't tested any of the code in this answer, so there may be bugs hiding in there. OK, first things first. Let's figure out what we're looking at: You want to write a function that solves a given instance of the traveling salesman problem. The algorithm you've decided to use is to enumerate the permutations of cities, terminating ...


4

As a brute force solution to the Traveling Salesman it seems to make sense. The algorithm seems to test every single combination of cities. The method used for listing remaining cities seems to be a simple one. You could avoid a lot of allocations and adding/removing to lists by using another method to track visited cities. For example, instead of creating ...


4

You could put the statuses into a constant array and use the index variable to fetch them out: void BlockParty::on_comboBox_currentIndexChanged(int index, Score* score) { static const BlockPartyLogic value_table[] = { BlockPartyLogic::RAMP_OFF, // Index 0 BlockPartyLogic::RAMP_PARTIAL, // Index 1 BlockPartyLogic::RAMP_COMPLETE, // ...


4

I have used a similar approach to implement MVC as you have Mantosh. I would however use references/pointers for both the view and model objects within the controller-class. This would then enable you to directly use the signal-slot mechanism and not need to set a controller in the viewer. This would also imply that the view and model are fully independent ...


Only top voted, non community-wiki answers of a minimum length are eligible