[mythtv] [PATCH] less crashes when no connection can be made to backend

[Tako Schotanus]

Isaac Richards wrote:

>On Wednesday 01 October 2003 06:14 am, Tako Schotanus wrote:
>  
>
>>Can you at least tell me what you mean by "fix your indentation"? As it
>>stands your remark is useless to me.
>>
>>Everything looks good on my end of things, but that's obvious of course.
>>So what's wrong? Am I using tabs instead of spaces? The other way
>>around? What?
>>    
>>
>
>Just look at the patch, and it should be pretty obvious.  When you're 
>contributing code to a project, you should try to copy the existing 
>indentation style, which you didn't do.
>
>For example:
>
>@@ -136,5 +136,9 @@
>             cerr << "You probably should modify the Master Server settings
>\n";
>             cerr << "in the setup program and set the proper IP address.\n";
>-            exit(0);
>+                       MythPopupBox::showOkPopup(mainWindow, tr("connection 
>failure"), tr("Could not connect to backend server"));
>+                       serverSock->close();
>+                       delete serverSock;
>+                       serverSock = NULL;
>+                       return false;
>         }
>     }
>  
>
True, there were several long lines. Fixed now.

>You even mixed tabs and spaces in code you added:
>
>+    const QObjectList *objlist = children();
>+    QObjectListIt it(*objlist);
>+    QObject *objs;
>+       int i = 0;
>+    while ((objs = it.current()) != 0)
>+    {
>
Ok, fixed (even some that weren't mine, including some "wrong" braces).
NB: Look how "obvious" the convention is:
     libs\libdvddev\dvbci.cpp - 59 tabs
     libmyth\audiooutputalsa.cpp - 14 tabs
     libmyth\mythplugin.cpp - 3 tabs
     libmyth\mythwizard.cpp - 111 tabs
     libmyth\oldsettings.cpp - 5 tabs
     libmyth\minitzo.cpp - 842 tabs

etc. etc. etc.

>All I ask is that people at least _try_ when submitting code for 
>consideration.  I already spent an hour or two reworking the last patch you 
>sent in, and I could have used that time for something more productive.
>
Spending 4 nights until deep in the night figuring out how the code
works so I can change it is "trying" in my book..

>  No 
>tabs.  4 space indents.  Braces on their own lines, aligned with the start of 
>the previous code block.  Generally no > 80 character lines, wrapping to the 
>start of a ( if possible.  No obvious comments.
>
>Example on that:
>
>+ * Adds a label of the given size to the pop-up.
>+ *   caption - The text to show in the label
>+ *   size - The size to use for the caption: Large, Medium or Small.
>+ * Returns the newly created QLabel
>+ */
>+QLabel *MythPopupBox::addLabel(QString caption, LabelSize size)
>
>That comment should be completely unnecessary -- the function name is 
>'addLabel', so it's obviously adding a label.  'caption' is obvious, 'size' 
>can only _be_ Large, Medium, or Small, and what other QLabel is it going to 
>return?
>
In my opinion you just don't write documentation for one function and 
not for the other just because "it's obvious".
If everybody would write extensive, even obvious, comments in the code 
you'd be halfway towards having a API reference manual.

>On an unrelated note, this:
>
>-    int width = (int)(800 * wmult);
>-    int height = (int)(600 * hmult);
>+    int width = (int)(m_parent->width() * wmult);
>+    int height = (int)(m_parent->height() * hmult);
>
>is wrong -- the parent's geometry has already been compensated for the actual 
>screen size, so multiplying it by wmult/hmult just throws stuff off.
>

Ok.

>And just a suggestion, but have you ever used default values for function 
>arguments?  You could get rid of about half of the functions you added to 
>MythPopupBox that way.
>
If you look at the methods you are referring to you can see that they
actually perform different functions depending on their arguments.
With your suggestion I'd have to if-statements to distinquish between
the cases, in my code the method overloading takes care of that.
Both are perfectly normal options, I just happen to prefer method
overloading over default values, that's all.

-Tako


-------------- next part --------------
A non-text attachment was scrubbed...
Name: noconnection.zip
Type: application/x-zip-compressed
Size: 7908 bytes
Desc: not available
Url : http://mythtv.org/pipermail/mythtv-dev/attachments/20031001/1eb93fce/noconnection.bin