DiveShare patch

[Salvo Tomaselli]

How about now?

I used astyle and I think I've addressed all the comments.
On 13/10/2014 13:03, Thiago Macieira wrote:
> Hi Salvo
>
> Thanks for the patch. The idea looks good and the networking part per se looks
> correct (even if I haven't run the code), but like Dirk said, it needs a
> couple of fixes before it goes in. Here are a couple of suggestions from me:
>
> On Sunday 12 October 2014 19:41:41 Salvo Tomaselli wrote:
>> +DiveShareExportDialog::DiveShareExportDialog(QWidget *parent) :
>> +       QDialog(parent),
>> +       ui(new Ui::DiveShareExportDialog)
>> +{
>> +       ui->setupUi(this);
>> +       this->nam = new QNetworkAccessManager(this);
> Please don't create your own QNetworkAccessManager. We have a global one in
> WebServices::manager(). Just reuse that.
>
>> +       QObject::connect(
>> +               this->nam, SIGNAL(finished(QNetworkReply*)),
>> +               this, SLOT(finishedSlot(QNetworkReply*))
>> +       );
>> +}
> Which in turn means your slot would be called for all the replies Subsurface
> is getting. It would be better if you connected to each QNetworkReply object
> you get instead of this global one.
>
>> +void DiveShareExportDialog::getUID() {
>> +       QDesktopServices::openUrl(QUrl(DIVESHARE_BASE_URI "/secret"));
>> +}
> Nitpick: here and in a couple of other functions, you have inconsistent style.
> The opening { for functions and global structs should go to the next line.
>
> I'd also recommend renaming this function. Even though "get" here is an action
> being taken, everyone reading the code will think of a getter function.
>
>> +QString generate_html_list(QByteArray data) {
> This function should be static.
>
>> +       if (reply->error() != 0) {
>> +               const char* error;
>> +               switch (reply->error()) {
> [snip]
>> +
>> this->ui->buttonBox->setStandardButtons(QDialogButtonBox::Cancel); +
>>         this->ui->txtResult->setHtml(QString(error));
> No need to do that large switch. Just use reply->errorString().
>
>> +       request.setRawHeader("User-Agent", "subsurface");
> Please use UserSurvey::getUserAgent().
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: diveshare.patch
Type: text/x-patch
Size: 18086 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20141013/c6452cb4/attachment.bin>