Thanks for those very helpful comments Bill. I’ve fixed up most of the badness in r5540. There remains one print to stdout that I’m using to watch the evolution of the scheduling array. I’ll take that out pretty soon. Otherwise, I think that I’ve addressed each of your concerns. Steve k9an > On Jun 5, 2015, at 11:05 AM, Bill Somerville <g4...@classdesign.com> wrote: > > On 05/06/2015 16:41, Steven Franke wrote: >> Bill, > Hi Steve, >>> It is only a small change. If you move the call of next_tx_state() from >>> MainWindow::bandHopping() into WSPRBandHopping::next_hop() just after >>> the call to FC_hopping(). The member variable m_->tx_percent_ tracks the >>> current value of the UI Tx Pct spin box. >>> >>> Something like: >>> >>> tx_next = next_tx_state (m_->tx_percent_); >>> >>> then revert MainWindow::bandHopping() to the commented out line above >>> the previous call to next_tx_state(). >> This is done and has been committed as r5539. Running it here now. > A couple of minor(ish) comments: > > WsprTxScheduler.h should be defining the API of your module, as such it > probably only needs to say: > > #ifndef WSPR_TX_SCHEDULER_H_ > #define WSPR_TC_SCHEDULER_H_ > > int next_tx_state (int pctx); > > #endif > > Note the header guards. > > For sure, putting 'using namespace std;' into a header file is very bad > practice and could break any code that includes it. > > The other includes and variables should go in the implementation file > where they are best declared as static since they do not need external > linkage. So only next_tx_state() needs external linkage unless any of > the other functions is intended to be used in other modules. > > The idiomatic C++ way of making definitions not have external linkage is > to put them inside the unnamed namespace. This is not just syntactic > sugar as it lets the compiler know that the definitions are not used > elsewhere so it can aggressively optimize the code in a way that it > couldn't without that knowledge. > > With that you then include WsprTxScheduler.h where it is going to be > used, i.e. in WSPRBandHopping.cpp. There is no need to declare the > function next_tx_state() as having external linkage since that is the > default achieved including the module header file as defined above. > > Writing to stdout from a routine that is going to be called from a GUI > application is probably not wise. I would suggest another public > function that returns a string with that information so that the caller > can decide what to do with the information or if it is only for testing > purposes you can disable it in release configuration builds by using the > NDEBUG preprocessor macro and #ifdef. >> Steve k9an > 73 > Bill > G4WJS. > > ------------------------------------------------------------------------------ > _______________________________________________ > wsjt-devel mailing list > wsjt-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/wsjt-devel
------------------------------------------------------------------------------ _______________________________________________ wsjt-devel mailing list wsjt-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/wsjt-devel
