• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 317
  • Last Modified:

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

0
Raul77
Asked:
Raul77
  • 7
  • 5
2 Solutions
 
anarki_jimbelCommented:
I can't really see any problems with the code.

However, do you really need the 'arraycount ' variable? You may get count as a property of the array therefore the line "           arraycount = arraycount + 1;" seems redundant for me.

Another moment.

The code
            Type oType = obj.GetType();
            string objType = oType.ToString();
can be written in one line because you don't really need oType variable. Can be:

 string objType = obj.GetType().ToString();
0
 
anarki_jimbelCommented:
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 :)
0
 
anarki_jimbelCommented:
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 :)
0
Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
Raul77Author Commented:
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,
0
 
Raul77Author Commented:
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.
0
 
anarki_jimbelCommented:
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! :)
0
 
Raul77Author Commented:
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.
0
 
anarki_jimbelCommented:
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...
0
 
Raul77Author Commented:
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

0
 
anarki_jimbelCommented:
I did't test it , of course, but I believe the function should be like in the first snippet. Hope I didn't miss any braces.

In the code (Page_Load) you call the function as:

                UserSummaryBox.Text += GenerateSummary(arraycount, ((Visitor)obj));

public string GenerateSummary(int arraycount, Visitor visitorObj){
   "\nVisitor " + arraycount + "\nIP Address: " + visitorObj.IPAddress + "\nBrowser: " + visitorObj.Browser + "\nUser Agent: " + visitorObj.UserAgent + "\n<b>Secured Connection:</b> " + visitorObj.IsSecureConnection + "\nPrefered Languages: " + visitorObj.PreferredLanguages + "\nRequest Type: " + visitorObj.RequestType + "\nLogon Time: " + visitorObj.TimeVisited;
}

Open in new window

0
 
Raul77Author Commented:
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.
0
 
anarki_jimbelCommented:
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!
0

Featured Post

What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

  • 7
  • 5
Tackle projects and never again get stuck behind a technical roadblock.
Join Now