RE: [VB.NET]Email Spammer[SOURCE] 04-03-2013, 12:41 PM
#8
(04-03-2013, 05:56 AM)ArkPhaze Wrote: Alright, since you asked for feedback on the code I wanted to go through the entire thing with you.
Where it starts...
Don't use End, it's a VB6 carry over method, meaning it is only there for compatibility.Code:Private Sub GhostButton3_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles GhostButton3.Click End End Sub
You don't need to declare the exception if you don't need to use it, although this is poor. If an exception is thrown because of cancelling, then you are probably not handling anything and just expecting to forcefully cancel the asynchronous operation, ignoring any errors.Code:Private Sub GhostButton2_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles GhostButton2.Click Try BackgroundWorker1.CancelAsync() Catch EX As Exception End Try End Sub
What is this BackgroundWorker here for?
Code:Private WithEvents TestWorker As System.ComponentModel.BackgroundWorker
Use AndAlso here to shortcircuit for better performance on the condition checking...Code:ElseIf GhostTextBox6.Text = "" And GhostTextBox7.Text = "" Then
Also in this conditional you are using MsgBox(), which is an ugly carry over from VB6 too.
You also declare this useless TestWorker that doesn't do anything as a new instance of the BackgroundWorker for no reason every time this button is clicked. Not only that, but the BackgroundWorker class implements the IDisposable interface for a reason, and you aren't disposing of ANY of these resources because none of these instances are being Dispose()'d of... This is bad, because when the GC comes along and does it's sweep, garbage collection is a resource intensive task, and it could be avoided more likely if you paid more attention to things like this.
Now, in your BackgroundWorker1_DoWork() event method, there are other classes that implement the IDisposable interface, but you aren't disposing of anything at all... I actually don't see you disposing of anything in this entire source code.
And before I explain about the ProgressBar, this entire bit of code at the end:
Code:Dim a, b As Integer Dim np As New System.Drawing.Point Private Sub GhostTheme1_MouseDown(ByVal sender As Object, ByVal e As System.Windows.Forms.MouseEventArgs) Handles GhostTheme1.MouseDown a = MousePosition.X - Me.Location.X b = MousePosition.Y - Me.Location.Y End Sub Private Sub GhostTheme1_MouseMove(ByVal sender As Object, ByVal e As System.Windows.Forms.MouseEventArgs) Handles GhostTheme1.MouseMove If e.Button = MouseButtons.Left Then np = MousePosition np.X = np.X - a np.X = np.Y - b End If End Sub
Is all useless. You are setting values that aren't being used anywhere else in your entire program, and the fact that they are member variables to your Form1 class, their scope is pretty widespread, and you're using up that extra memory on the stack for no reason (because Integer and System.Drawing.Point are value types).
Now as for the ProgressBar, you simply can't update the ProgressBar because a BackgroundWorker runs asynchronously on a new thread. How can you update the ProgressBar, which was created on the UI thread, from a BackgroundWorker thread? You need to invoke the UI thread to do anything with the ProgressBar.
Thank you for your feedback.
To your first feedback that is why I did use "End", well I dont think this does any harm as it is acceptable in VB.NET though it was derived from VB6 , but from time I will not use this for sure following you.So I will use "Application.Exit()", I hope using this is okay.
Secondly,
for the try catch one, I agree to your point and thanks for clearing the concept.
Thirdly,
Code:
Private WithEvents TestWorker As System.ComponentModel.BackgroundWorkerfourthly,
Yes, I will use AndAlso , I didnt knew about this, thanks for pointing out.
fifthly,
I don't see any harm in using MsgBox(), it works perfectly.I think thats more important.
Sixth,
Code:
Dim a, b As Integer
Dim np As New System.Drawing.Point
Private Sub GhostTheme1_MouseDown(ByVal sender As Object, ByVal e As System.Windows.Forms.MouseEventArgs) Handles GhostTheme1.MouseDown
a = MousePosition.X - Me.Location.X
b = MousePosition.Y - Me.Location.Y
End Sub
Private Sub GhostTheme1_MouseMove(ByVal sender As Object, ByVal e As System.Windows.Forms.MouseEventArgs) Handles GhostTheme1.MouseMove
If e.Button = MouseButtons.Left Then
np = MousePosition
np.X = np.X - a
np.X = np.Y - b
End If
End SubThe above code is my doubt number 2, But I had edited the thread and removed that part.The Source has been edited Now, I have used different functions now, removed MsgBox and End
Thread Updated and progress bar fixed.
Thank you for your suggesttion.
![[Image: OilyCostlyEwe.gif]](http://fat.gfycat.com/OilyCostlyEwe.gif)
![[+]](https://sinister.ly/images/modern/collapse_collapsed.png)