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
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);
}
}
SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
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 :)
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).IsSecureC onnection + "\nPrefered Languages: " + ((IEVisitor)obj).Preferred Languages + "\nRequest Type: " + ((IEVisitor)obj).RequestTy pe + "\nLogon Time: " + ((IEVisitor)obj).TimeVisit ed;
}
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).IPAddr ess + "\nBrowser: " + ((NonIEVisitor)obj).Browse r + "\nUser Agent: " + ((NonIEVisitor)obj).UserAg ent
+ "\n<b>Secured Connection:</b> " + ((NonIEVisitor)obj).IsSecu reConnecti on + "\nPrefered Languages: " + ((NonIEVisitor)obj).Prefer redLanguag es + "\nRequest Type: " + ((NonIEVisitor)obj).Reques tType + "\nLogon Time: " + ((NonIEVisitor)obj).TimeVi sited;
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,
if (objType == "IEVisitor")
{
totalIE = totalIE + 1;
UserSummaryBox.Text += "\nVisitor " + arraycount + "\nIP Address: " + ((IEVisitor)obj).IPAddress
+ "\nSecured Connection: " + ((IEVisitor)obj).IsSecureC
}
since i know the object is IEVisitor , cant i cast it somehow once instead of writing
((IEVisitor)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).IPAddr
+ "\n<b>Secured Connection:</b> " + ((NonIEVisitor)obj).IsSecu
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,
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.
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! :)
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! :)
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.
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...
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...
ASKER
this is my visitor class.
btw i tried doing this:
but no luck ....
ArrayList visitors = (ArrayList)Application["Vi sitor"];
foreach (Object obj in visitors)
{
arraycount = arraycount + 1;
if (obj is IEVisitor)
{
IEVisitor visitor = (IEVisitor)obj;
totalIE = totalIE + 1;
}
else
{
if (((NonIEVisitor)obj).Brows er == "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;
}
btw i tried doing this:
but no luck ....
ArrayList visitors = (ArrayList)Application["Vi
foreach (Object obj in visitors)
{
arraycount = arraycount + 1;
if (obj is IEVisitor)
{
IEVisitor visitor = (IEVisitor)obj;
totalIE = totalIE + 1;
}
else
{
if (((NonIEVisitor)obj).Brows
{
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
}
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();
}
}
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
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!
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!
However, in this case I'd use simple for statement, not 'for each '. But this is matter of taste :)