Question

I'm building an audit trail in C#, asp.net. On the .aspx page I have several hidden fields which associate themselves with regular fields (i.e. txtFirstName -> firstNameTrackerHiddenField). I have it set up so that when the user inputs/selects data to a field, the hidden field gets a value set to it. Example:

    protected void txtFirstName_TextChanged(object sender, EventArgs e)
    {
        this.FirstNameTrackerHiddenField.Value = "1";
    }

Now to build my Audit Log I was thinking of just checking through each hidden field, pulling the ones that aren't null, and appending a string, depending on which field the user has inputed/changed. I could do a bunch of nested if statements but that would be really sloppy. Sorry if I'm not making any sense, I'm still a little new to the coding world and I thought this methodology would be pretty neat and easy to implement. Here's what I have so far in the auditLog method:

    protected string auditLogString()
    {
        string auditLog = this.txtAuditLogReadOnly.Text + System.Environment.NewLine + System.Environment.NewLine + 
            DateTime.Now.ToString() + "- (Incident saved by: " + Page.User.Identity.Name + ") ";

        if (this.FirstNameTrackerHiddenField.Value != string.Empty)
        {
            auditLog += "- with changes to First Name."
            if (this.LastNameTrackerHiddenField.Value != string.Empty)
            {
                auditLog += "- with changes to Last Name."
            }
        }

        return auditLog;
    }

And the list goes on. There's about 50 fields to cycle through, so that's why I was wondering if there was a better way to go about this... Thanks for any input, and please don't be rough with me... :)

Was it helpful?

Solution

A cleaner way to if/else-ing every HiddenField would be to loop through the controls on your page, check their type and if they're a HiddenField with a value, do something with the value.

foreach (Control ctrl in this.Controls)
{
    if (ctrl is HiddenField)
    {
        if (!string.IsNullOrEmpty((ctrl as HiddenField).Value))
        {
            // do something
        }
    }
}

OTHER TIPS

Ok, so a few things here. First - try not to do string concatenation using + in C#, unless you're concatenating constant expressions (which DateTime.Now and Page.User.Identity.Name are not). Use a StringBuilder instead. Strings in C# are immutable.

Second, you can just add a custom attribute to your textboxes and collect their value server-side; there's no need for additional hidden fields.

So, given an example form like: -

        <asp:TextBox ID="txtAuditLogReadOnly" runat="server" />
        <div id="divContainer" runat="server">

            <asp:TextBox ID="t1" runat="server" fieldname="First Name" />
            <asp:TextBox ID="t2" runat="server" fieldname="Last Name" />
            <asp:TextBox ID="t3" runat="server" fieldname="Shoe Size" />
            <asp:TextBox ID="t4" runat="server" fieldname="Banana" />

        </div>

(Note 'fieldname' attribute) You could scrape the values into your audit log like so: -

        var builder = new StringBuilder(
            string.Format("{0}{1}{2:dd/MM/yyyy hh:mm}- (Incident saved by: {3})",
                txtAuditLogReadOnly.Text,
                Environment.NewLine,
                DateTime.Now,
                Page.User.Identity.Name));

        var controls = from Control c in divContainer.Controls
                       select c;

        foreach (var ctl in controls)
        {
            if (ctl is TextBox)
            {
                var txt = (TextBox)ctl;

                if (!string.IsNullOrEmpty(txt.Text))
                {
                    string fieldname = txt.Attributes["fieldname"];
                    builder.AppendFormat(" - with changes to {0}", fieldname);
                }
            }
        }

        return builder.ToString();
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top