Link to home
Start Free TrialLog in
Avatar of Raul77
Raul77

asked on

Can somone please help clearing this code?

Hi again !
I have the following code which works but since i just started learning C# i rather learn it the correct way, somehow i think i am doing too much work here and there is a shorter way to do this? can someone perhaps explain where i am repeating myself? or how i can make it shorter?

Thank you
public partial class UserSummary : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {
        int totalIE = 0;
        int totalFF = 0;
        int arraycount = 0;
        int totalOthers = 0;
        if (Application["visitor"] == null)
        { Response.Redirect("Login.aspx"); }
        ArrayList visitors = (ArrayList)Application["Visitor"];         
        foreach (Object obj in visitors)
        {
       
            arraycount = arraycount + 1;
            Type oType = obj.GetType();
            string objType = oType.ToString();
            if (objType == "IEVisitor")
            {
                totalIE = totalIE + 1;
                UserSummaryBox.Text += "\nVisitor " + arraycount + "\nIP Address: " + ((IEVisitor)obj).IPAddress + "\nBrowser: " + ((IEVisitor)obj).Browser + "\nUser Agent: " + ((IEVisitor)obj).UserAgent
                    + "\n<b>Secured Connection:</b> " + ((IEVisitor)obj).IsSecureConnection + "\nPrefered Languages: " + ((IEVisitor)obj).PreferredLanguages + "\nRequest Type: " + ((IEVisitor)obj).RequestType + "\nLogon Time: " + ((IEVisitor)obj).TimeVisited;
            }
            else
            {
                if (((NonIEVisitor)obj).Browser == "Firefox")
                {
                    totalFF = totalFF + 1;
                    UserSummaryBox.Text += "\nVisitor " + arraycount + "\nIP Address: " + ((NonIEVisitor)obj).IPAddress + "\nBrowser: " + ((NonIEVisitor)obj).Browser + "\nUser Agent: " + ((NonIEVisitor)obj).UserAgent
                       + "\n<b>Secured Connection:</b> " + ((NonIEVisitor)obj).IsSecureConnection + "\nPrefered Languages: " + ((NonIEVisitor)obj).PreferredLanguages + "\nRequest Type: " + ((NonIEVisitor)obj).RequestType + "\nLogon Time: " + ((NonIEVisitor)obj).TimeVisited;
 
                }
                else
                {
                    totalOthers = totalOthers + 1;
                    UserSummaryBox.Text += "\nVisitor " + arraycount + "\nIP Address: " + ((NonIEVisitor)obj).IPAddress + "\nBrowser: " + ((NonIEVisitor)obj).Browser + "\nUser Agent: " + ((NonIEVisitor)obj).UserAgent
                   + "\n<b>Secured Connection:</b> " + ((NonIEVisitor)obj).IsSecureConnection + "\nPrefered Languages: " + ((NonIEVisitor)obj).PreferredLanguages + "\nRequest Type: " + ((NonIEVisitor)obj).RequestType + "\nLogon Time: " + ((NonIEVisitor)obj).TimeVisited;
                }
            }
                      
               }
        Response.Write("<h1>" + "Visitor Summary Page" + "</h1>");
        Response.Write("<b>" + "Total IE Visitors: "+"</b>"  + totalIE);
        Response.Write("<b>" + "<br>Total FireFox Visitos: " + "</b>" + totalFF);
        Response.Write("<b>" + "<br>Total Other Visitors: " + "</b>" + totalOthers);
    }
}

Open in new window

SOLUTION
Avatar of Dmitry G
Dmitry G
Flag of New Zealand image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
My apologies, you are using arraycount variable in the code, forget about my first remark.
However, in this case I'd use simple for statement, not 'for each '. But this is matter of taste :)
And on more. I just wonder myself  - what's better: to call Response.Write four times of prepare one big string and then call Response.Write just once? I have a suspicion the las would be preferrable but I hope ASP guru can correct me if I'm wrong :)
Avatar of Raul77
Raul77

ASKER

thanks, what i was thinking is lets take this line for example

if (objType == "IEVisitor")
            {
                totalIE = totalIE + 1;
                UserSummaryBox.Text += "\nVisitor " + arraycount + "\nIP Address: " + ((IEVisitor)obj).IPAddress + "\nBrowser: " + ((IEVisitor)obj).Browser + "\nUser Agent: " + ((IEVisitor)obj).UserAgent
                    + "\nSecured Connection: " + ((IEVisitor)obj).IsSecureConnection + "\nPrefered Languages: " + ((IEVisitor)obj).PreferredLanguages + "\nRequest Type: " + ((IEVisitor)obj).RequestType + "\nLogon Time: " + ((IEVisitor)obj).TimeVisited;
            }

since i know the object is IEVisitor , cant i cast it somehow once instead of writing

((IEVisitor)obj).IPAddress just use obj.IPAddress

right now i cast it for every property. also is it possible to create a method to not type the whole  

UserSummaryBox.Text += "\nVisitor " + arraycount + "\nIP Address: " + ((NonIEVisitor)obj).IPAddress + "\nBrowser: " + ((NonIEVisitor)obj).Browser + "\nUser Agent: " + ((NonIEVisitor)obj).UserAgent
                   + "\n<b>Secured Connection:</b> " + ((NonIEVisitor)obj).IsSecureConnection + "\nPrefered Languages: " + ((NonIEVisitor)obj).PreferredLanguages + "\nRequest Type: " + ((NonIEVisitor)obj).RequestType + "\nLogon Time: " + ((NonIEVisitor)obj).TimeVisited;

and perhaps just call a method? its just looks like too much typing for each if else, since i am spitting out the same info would a method be possible?

Thanks,
Avatar of Raul77

ASKER

i actually did this and it worked !

            if (obj is IEVisitor)
            {
                IEVisitor ieVisitor = (IEVisitor)obj;

this way i dont even need gettype line and no need to cast each like.
Yeah, you are right. I didn't probably understood your intentions. Of course it's much better to cast the object once rather than when getting each property.

And probably I'd move the code to prepare user summary to a separate function as you suggested. However it won't make the code faster. But definitely more readable.

So - you know everything yourself! :)
Avatar of Raul77

ASKER

Thanks, can you help me start writing a function? i am trying to write one function . but my problem is how do I pass the type to the function so i can cast it based on the type ? point is yours if you can help me started.
appreciate it.
OK, not a problem but have to think a bit.

As I understand, NonIEVisitor and IEVisitor are custom types. May be they implement the same interface that has all this set of propertie4s and functions like "IsSecureConnection ", "RequestType" etc? In this case it would be much easier to write the functions as we may cast to an interface type...
Avatar of Raul77

ASKER

this is my visitor class.
btw i tried doing this:
but no luck ....

       ArrayList visitors = (ArrayList)Application["Visitor"];        
        foreach (Object obj in visitors)
        {
            arraycount = arraycount + 1;
            if (obj is IEVisitor)
            {
                IEVisitor visitor = (IEVisitor)obj;
                totalIE = totalIE + 1;
                               
            }
            else
            {
                if (((NonIEVisitor)obj).Browser == "Firefox")
                {
                    NonIEVisitor visitor = (NonIEVisitor)obj;
                    totalFF = totalFF + 1;      
                   
                }
                else
                {
                    NonIEVisitor visitor = (NonIEVisitor)obj;
                    totalOthers = totalOthers + 1;
                   
                }
               
            }
            UserSummaryBox.Text += "\nVisitor " + arraycount + "\nIP Address: " + visitor.IPAddress + "\nBrowser: " + visitor.Browser + "\nUser Agent: " + visitor.UserAgent
           + "\n<b>Secured Connection:</b> " + visitor.IsSecureConnection + "\nPrefered Languages: " + visitor.PreferredLanguages + "\nRequest Type: " + visitor.RequestType + "\nLogon Time: " + visitor.TimeVisited;
           
               }
public abstract class Visitor : IComparable
{
    public string IPAddress { get; set; }           // IP Address 
    public string UserAgent { get; set; }           // type of client
    public string Browser { get; set; }             // browser name
    public DateTime TimeVisited { get; set; }       // time visited 
    public string RequestType { get; set; }         // get or post
    public string[] PreferredLanguages { get; set; }  // list of perfered languages
    public bool IsSecureConnection { get; set; }    // http or https
 
	public Visitor()
	{
		//
		// TODO: Add constructor logic here
		//
	}
 
    public Visitor( string ipAddress, 
                    string userAgent,
                    string browser,
                    string requestType,
                    string[] preferredLanguages,
                    bool isSecureConnection)
    {
        TimeVisited = DateTime.Now;  // set the time to now
        IPAddress = ipAddress;
        UserAgent = userAgent;
        Browser = browser;
        RequestType = requestType;
        PreferredLanguages = (string[])preferredLanguages.Clone();
        IsSecureConnection = isSecureConnection;
    }
 
    #region IComparable Members
 
    public int CompareTo(object obj)
    {
        if (obj is Visitor)
        {
            Visitor otherVisitor = (Visitor)obj;
            return this.TimeVisited.CompareTo(otherVisitor.TimeVisited);
        }
        else
        {
            throw new ArgumentException("Object is not a Visitor");
        }
    }
 
    #endregion
}
 
// for IE visitors
public class IEVisitor : Visitor
{
    public string FilePath { get; set; }
 
    public IEVisitor(string ipAddress,
                     string userAgent,
                     string browser,
                     string requestType,
                     string[] preferredLanguages,
                     bool isSecureConnection,
                     string filePath)
        : base(ipAddress, userAgent, browser, requestType, preferredLanguages, isSecureConnection)
    {
        FilePath = filePath;
    }
 
    // display the details in HTML table
    public override string ToString()
    {
        System.Text.StringBuilder objInTable = new System.Text.StringBuilder();
        objInTable.Append("<table>");  // NOT COMPLETED!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
        return objInTable.ToString();
    }
}
 
 
// for non IE visitors
public class NonIEVisitor : Visitor
{
    public System.Collections.Specialized.NameValueCollection QueryString { get; set; }
 
    public NonIEVisitor(string ipAddress,
                     string userAgent,
                     string browser,
                     string requestType,
                     string[] preferredLanguages,
                     bool isSecureConnection,
                     System.Collections.Specialized.NameValueCollection queryString)
        : base(ipAddress, userAgent, browser, requestType, preferredLanguages, isSecureConnection)
    {
        QueryString = (System.Collections.Specialized.NameValueCollection)queryString;
    }
 
    // display the details in HTML table
    public override string ToString()
    {
        System.Text.StringBuilder objInTable = new System.Text.StringBuilder();
        objInTable.Append("<table>");  // NOT COMPLETED!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
        return objInTable.ToString();
    }
}

Open in new window

ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of Raul77

ASKER

Thanks, everything worked except i think you missed "return" on the function which i added manually. awesome, i just started learning and appreciate the help.
You are right - I missed return. Shame on me! :)

By the way, your example is a good demonstration of why we might need abstract classes or interfaces. In your code you need to know that the object is of the Visitor type to invoke properties object properties. And it does not really matter if it is IEVisitor or NonIEVisitor. Saves a lot of efforts.
Good luck!