2012-04-10 4 views
0

Ich versuche, die Liste der Elemente in meinem E-Mail-Textkörper über meine emial-Setup-Klasse aufzufüllen. Wenn ich versuche, E-Mail in meinem Outlook zu öffnen, sehe ich nur ein Element, wo ich eine Liste von Elementen erwarte.Liste der Elemente in E-Mail-Nachrichtentext ausfüllen

Unten ist mein Code:

public class EmailSetup 
{ 
    string toEmailSetup = string.Empty; 
    string fromEmailSetup = string.Empty; 
    string domainName = string.Empty; 
    string emailServer = string.Empty; 

    public void ApplicationFailedEmailSetup(List<string>ApplicationsInactive,DateTime dateRun) 
    { 
     toEmailSetup = ConfigurationManager.AppSettings["To mailid"]; 
     fromEmailSetup = ConfigurationManager.AppSettings["From mailid"]; 
     domainName = ConfigurationManager.AppSettings["Domain Name"]; 
     emailServer = ConfigurationManager.AppSettings["Email Server"]; 
     try 
     { 
      var messager = new MailMessage(); 
      messager.To.Add(toEmailSetup); 
      messager.Subject = "Applications Crashed/Closed"; 
      messager.From = new MailAddress(fromEmailSetup); 
      try 
      { 
       messager.Body = "Following applications you are monitoring are closed are crashed:"; 
       **foreach (var item in ApplicationsInactive) 
       { 
        messager.Body = item; 
       }** // Here i am trying to populate list of applications. 
      } 
      catch (Exception) 
      { 
       throw; 
      } 

      var smtp = new SmtpClient(emailServer); 
      smtp.EnableSsl = true; 

      try 
      { 
       smtp.DeliveryMethod = SmtpDeliveryMethod.Network; 
       smtp.UseDefaultCredentials = false; 
       smtp.Send(messager); 
      } 
      catch (Exception) 
      { 

       throw; 
      } 

     } 
     catch (SmtpException ex) 
     { 
      throw new ApplicationException 
       ("SmtpException has occured: " + ex.Message); 
     } 

    } 

} 

Antwort

4

Diese Zeile in der Schleife ist das Problem:

messager.Body = item; 

Sie überschreiben die Body Eigenschaft jedes Mal, wird so nur das letzte Element der Schleife folgt dort sein. Sie wollen stattdessen anfügen:

messager.Body += item; 

Es gibt mehr Möglichkeiten, dies zu tun, natürlich, und dies ist eigentlich ein bisschen schlampig. Sehen Sie sich die Klasse StringBuilder an, um formatierte Zeichenfolgen zu erstellen und Ihren E-Mail-Textkörper zu erstellen. Stellen Sie dann einfach den E-Mail-Text auf .ToString() des Objekts StringBuilder ein.


Auch als eine Randnotiz, dieser Code dient Ihnen keinen Zweck:

catch (Exception) 
{ 
    throw; 
} 

Wenn Sie nicht tatsächlich die Ausnahme in sinnvoller Weise der Handhabung, warum es überhaupt fangen? Der Code wird die Ausnahme auslösen, also lass ihn einfach die Ausnahme auslösen. Es gibt absolut keinen Grund, es hier zu erfassen, und das erzeugt nur Rauschen im Code.

Zusätzlich ist auch dieser schlecht:

catch (SmtpException ex) 
{ 
    throw new ApplicationException 
     ("SmtpException has occured: " + ex.Message); 
} 

Sie die ursprüngliche Ausnahme zu unterdrücken und eine völlig neue zu erstellen. Sie verlieren den Stack-Trace und andere nützliche Informationen aus der ursprünglichen Ausnahme. Gibt es einen bestimmten Grund, dass Sie SmtpException s zu ApplicationException s konvertieren möchten? Stellen Sie die InnerException -Eigenschaft des ApplicationException zu dem SmtpException mindestens so ein, damit Sie diese Information nicht vollständig verlieren.

Aber mehr auf den Punkt, genau wie oben, behandeln Sie nicht wirklich die Ausnahme in irgendeiner sinnvollen Weise. Es wird kein Kontext hinzugefügt, keine Protokollierung wird ausgeführt und Sie werfen trotzdem eine Ausnahme aus. Auch dies ist nur Rauschen im Code. Es gibt keinen Grund, eine Ausnahme zu machen, wenn Sie nicht wirklich damit umgehen.

+0

Danke für die Antwort und danke für Korrigieren meines Codes. Ich bin in der Lage, Liste der Elemente zu füllen, aber alle kommen in nur einer Zeile. – 62071072SP

+1

@ 62071072: Hier kommt zusätzliche Formatierung ins Spiel. Wie Sie damit umgehen, hängt davon ab, ob Ihre E-Mail HTML ist oder nicht. Ich sehe HTML nicht in Ihrem Code, also nehme ich nicht an. Aber Sie können versuchen, für den Anfang etwas so einfaches wie: 'messager.Body + = Element + Environment.NewLine;' – David

3

Ihre aktuellen Code wird in jeder Iteration den Körper zu überschreiben, ist es jedes Element zuweisen - in dem letzten Element entstehende Körper.

Sie müssen append auf den Körper statt:

messager.Body = "Following applications you are monitoring are closed are crashed:"; 
messager.Body += string.Join(", ", ApplicationsInactive); 

Notiere die + = Operator statt nur =.

Sie brauchen auch keine Schleife - verwenden Sie einfach die handliche Join() Methode der string Klasse, um das gleiche Ergebnis mit weniger und besser lesbarem Code zu erreichen.

+0

ApplicationsInactive ist eine Liste, ich denke, ich kann keine Liste zur Join() -Methode hinzufügen – 62071072SP

+0

@ 62071072 Sie können mit [.NET 4.0] (http://msdn.microsoft.com/en-us/library/system .string.join (v = vs.100) .aspx) - im Falle eines älteren Frameworks ändern Sie einfach in 'string.Join (", ", ApplicationsInactive.ToArray());' –

+0

Vielen Dank für Ihre Antwort. Aber alle Punkte sind in einer Zeile eingetragen. Bsp .: Folgende Anwendungen sind überwacht, sind geschlossen sind abgestürzt: abc, akdksa, aodjal – 62071072SP

Verwandte Themen