0

emialセットアップクラスを使用して、メール本文にアイテムのリストを入力しようとしています。Outlookでメールを開こうとすると、アイテムのリストを期待しているアイテムが1つだけ表示されます。

以下は私のコードです：

 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);
        }

    }

}
4

2 に答える 2

4

ループ内のこの行が問題です。

messager.Body = item;

Body毎回プロパティを上書きしているので、ループの後に最後のアイテムだけが表示されます。代わりに追加したい：

messager.Body += item;

もちろん、これを行う方法は他にもありますが、これは実際には少しずさんです。クラスを見て、StringBuilderフォーマットされた文字列を作成し、メール本文を作成してから、メール本文をオブジェクトのに設定.ToString()します。StringBuilder

また、補足として、このコードは目的を果たしていません。

catch (Exception)
{
    throw;
}

実際に意味のある方法で例外を処理していないのなら、なぜそれをまったくキャッチしないのですか？コードは例外をスローするので、例外をスローさせます。ここでそれを捕まえる理由はまったくありません、そしてこれはコードにノイズを作成しているだけです。

さらに、これも悪いです：

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

元の例外を抑制し、まったく新しい例外を作成しています。元の例外からスタックトレースやその他の有用な情報が失われています。SmtpExceptionsをsに変換したい特別な理由はありますApplicationExceptionか？少なくとも、その情報が完全に失われないように、のInnerExceptionプロパティをに設定してください。ApplicationExceptionSmtpException

ただし、さらに重要な点として、上記のように、実際には意味のある方法で例外を処理しているわけではありません。コンテキストは追加されておらず、ロギングも行われていません。とにかく例外をスローします。繰り返しますが、これはコード内の単なるノイズです。実際に処理しない場合は、例外をキャッチする理由はありません。

于 2012-04-10T14:04:14.273 に答える
3

現在のコードは、各反復で本文を上書きし、各項目を割り当てています。その結果、最後の項目が本文になります。

代わりに本文に追加する必要があります。

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

=の代わりに+=演算子に注意してください。

また、ループはまったく必要ありません。クラスの便利なJoin()メソッドを使用するだけstringで、より読みやすいコードで同じ結果を得ることができます。

于 2012-04-10T13:59:39.580 に答える