C# Need help with an "if" statement.

Very basic here, a program that sends an email from either a "Gmail" or a "Live" account, depending on which radio button is clicked. Program works, but it keeps running both lines of code anyway...


if (radioButton1.Enabled == true && radioButton2.Enabled || true)
try
[
[

MailMessage message = new MailMessage();
message.From = new MailAddress(textBox6.Text);
message.Subject = textBox5.Text;
message.Body = textBox2.Text;
foreach (string s in textBox1.Text.Split(';'))
message.To.Add(s);
SmtpClient client = new SmtpClient();
client.Credentials = new NetworkCredential(textBox6.Text, textBox4.Text);
client.Host = "smtp.gmail.com";
client.Port = 587;
client.EnableSsl = true;
client.Send(message);

]
]
catch
[
]
finally
[
MessageBox.Show("Message sent!", "Success!");
]
if (radioButton2.Enabled == true && radioButton1.Enabled || true)
try
[
[

MailMessage message1 = new MailMessage();
message1.From = new MailAddress(textBox6.Text);
message1.Subject = textBox5.Text;
message1.Body = textBox2.Text;
foreach (string s in textBox1.Text.Split(';'))
message1.To.Add(s);
SmtpClient client1 = new SmtpClient();
client1.Credentials = new NetworkCredential(textBox6.Text, textBox4.Text);
client1.Host = "smtp.live.com";
client1.Port = 587;
client1.EnableSsl = true;
client1.Send(message1);
]
]
catch
[
]
finally
[
MessageBox.Show("Message sent!", "Success!");
]

You should be using != and not ||.

|| is logical or. It will always evaluate to true if you have:

somestatement || true

Originally posted by xrevxoltx
You should be using != and not ||.

|| is logical or. It will always evaluate to true if you have:

somestatement || true

What he said. Excellent catch in record time xrevoltx, I had to re-read through the code 3 times before I caught the minor technical error.

"if (radioButton1.Enabled == true && radioButton2.Enabled || true)"

As above said, you should be using "!=" which means "not equal to".

"||" means "or" and is used in the same way as "&&".

Additionally the == true is not necessary, radioButton1.Enabled returns a boolean, so just do this:

"if (radioButton1.Enabled && !radioButton2.Enabled)"

Although only one radiobutton should be enabled at any time, so just this should do:

"if (radioButton1.Enabled) [
// 1
] else [
// 2
]

Hmm, It keeps running both blocks of code.

Or you can simply use an inline if on the assignment of the SMTP host:

if (radioButton1.Enabled || radioButton2.Enabled) [
MailMessage message = new MailMessage();
message.From = new MailAddress(textBox6.Text);
message.Subject = textBox5.Text;
message.Body = textBox2.Text;
foreach (string s in textBox1.Text.Split(';'))
message.To.Add(s);
SmtpClient client = new SmtpClient();
client.Credentials = new NetworkCredential(textBox6.Text, textBox4.Text);
client.Host = radioButton1.Enabled ? "smtp.gmail.com" : "smtp.live.com";
client.Port = 587;
client.EnableSsl = true;
client.Send(message);
]

Instead of duplicating the above block of code.

reply to post by xrevxoltx


Forgive my ignorance, but I'm just starting to get into this. Obviously, your way looks far superior to running two blocks, but explain this line to me:

client.Host = radioButton1.Enabled ? "smtp.gmail.com" : "smtp.live.com";

so what I want is for radioButton1 to send to gmail and radioButton2 to send to live. I'd still also like to understand why both blocks keep executing even though the "IF's" aren't properly met.

That is what is bugging me so much. I though that "IF's" worked IF and ONLY IF the (parameters) are met, running the following
[
]
block of code

reply to post by graphuto


"if (radioButton1.Enabled) [
// 1
] else [
// 2
]

Use this like I suggested, then it can't possibly run both.

reply to post by graphuto


Maybe its an issue with the try? Comment it out with the comment tool if using VisualStudio and try it.

BTW, I think the "||" processes the first statement and if its true, the second is not further checked. If you want to use a true or use the "|"

Originally posted by graphuto
reply to post by xrevxoltx

 

Forgive my ignorance, but I'm just starting to get into this. Obviously, your way looks far superior to running two blocks, but explain this line to me:

client.Host = radioButton1.Enabled ? "smtp.gmail.com" : "smtp.live.com";

so what I want is for radioButton1 to send to gmail and radioButton2 to send to live. I'd still also like to understand why both blocks keep executing even though the "IF's" aren't properly met.

That is what is bugging me so much. I though that "IF's" worked IF and ONLY IF the (parameters) are met, running the following
[
]
block of code

edit on 12-5-2012 by graphuto because: (no reason given)

client.Host = radioButton1.Enabled ? "smtp.gmail.com" : "smtp.live.com";

Is equivalent to:

if (radioButton1.Enabled == true)
client.Host = "smtp.gmail.com";
else
client.Host = "smtp.live.com";

Originally posted by graphuto
reply to post by xrevxoltx

 

Forgive my ignorance, but I'm just starting to get into this. Obviously, your way looks far superior to running two blocks, but explain this line to me:

client.Host = radioButton1.Enabled ? "smtp.gmail.com" : "smtp.live.com";

so what I want is for radioButton1 to send to gmail and radioButton2 to send to live. I'd still also like to understand why both blocks keep executing even though the "IF's" aren't properly met.

That is what is bugging me so much. I though that "IF's" worked IF and ONLY IF the (parameters) are met, running the following
[
]
block of code

edit on 12-5-2012 by graphuto because: (no reason given)

If_this_is_true ? Do_this : Else_do_this

Ok, I understand what everyone is saying here, thanks for the inline function thing btw.

here is the current code:

if (radioButton1.Enabled || radioButton2.Enabled)
try
[
[

MailMessage message = new MailMessage();
message.From = new MailAddress(textBox6.Text);
message.Subject = textBox5.Text;
message.Body = textBox2.Text;
foreach (string s in textBox1.Text.Split(';'))
message.To.Add(s);
SmtpClient client = new SmtpClient();
client.Credentials = new NetworkCredential(textBox6.Text, textBox4.Text);
client.Host = radioButton1.Enabled ? "smtp.gmail.com" : "smtp.live.com";
client.Port = 587;
client.EnableSsl = true;
client.Send(message);

]
]
catch
[
]
finally
[
MessageBox.Show("Message sent!", "Success!");
]


Now the LIVE portion isn't working at all, it won't send the ones from Live

Only one radio button is selected at a given time, correct? I recall in VB 6 you had to group them somehow, not sure about C#.

ETA: Group them to ensure selecting one deselected the other, I mean.

Awesome, I was using wrong syntax. its radiobutton.CHECKED not ENabled

Thanks for all the awesome help though!

Now if someone could tell me how to add a sweet Icon and a (icon? the picture that shows in the taskbar when the program is running)

Originally posted by xrevxoltx
Only one radio button is selected at a given time, correct? I recall in VB 6 you had to group them somehow, not sure about C#.

ETA: Group them to ensure selecting one deselected the other, I mean.

edit on 12-5-2012 by xrevxoltx because: (no reason given)

VB6 and all the .net stuff are so vastly different it annoys me.

I was never formally taught any language, it's all been ad hoc when needed, so half the things I read in this thread - while perhaps following a standard - just seem so redundant for what is required.

Im a spaghetti coder... worst kind. and proud when I get things to work

(sorry, no help OP, but I'd have done it entirely different, and well, wont provide code as it's always poo poo'd when I do lol)

So now I understand it all perfectly. Of course it was running both blocks of code always, because those radiobuttons were always "Enabled" i.e., clickable, useable.

haha i feel so dumb. This has been bothering me for about 12 hrs now, I went to a party and all I could think about was this dumb code... lol

Stars for all you guys! Thanks so much for the tips!

reply to post by graphuto


Maybe live is use different client parameters such as port or an other outgoing server, different to the incoming server.
My advice is, you should create an 2D array and store all account informations there. Then, depending on wich radio button you click (or checkboxes, if you want to send simultaneos on different accounts), you select the content of the array.

create an array in the onclick method thats fed from the textboxes or store the infos in the code.
then use the index of the array in your if, or better switch it.

example:
client.Host = AccountArray[x,1];
client.Port = AccountArray[x,2];



edit: Radiobuttons should always sit in a groupbox.

Originally posted by graphuto
Awesome, I was using wrong syntax. its radiobutton.CHECKED not ENabled

Thanks for all the awesome help though!

Now if someone could tell me how to add a sweet Icon and a (icon? the picture that shows in the taskbar when the program is running)

edit on 12-5-2012 by graphuto because: (no reason given)

Wow I didn't even pick up on that. There should be a property to set the icon when you select the form. Assuming you're using Visual studio.

Yea I see the "Icon" one there, and it's set in there, but the "running tasK" down in the bar is that uggo sheet of paper looking icon, like one of those "unrecognized file type" icons.

Originally posted by graphuto
Now if someone could tell me how to add a sweet Icon and a (icon? the picture that shows in the taskbar when the program is running)

edit on 12-5-2012 by graphuto because: (no reason given)

I dont know the code for it currently, but you can select your Form and go to the Properties. There should be a propertie called "Form.Icon" there you can select any icon I think. It will be added to the ressources. (Using VisualStudio)

Edit: I created a fresh form and tried it with the win7 taskmanager icon in c:windowssystem32, it works for me.