|
try { ... } catch { throw; }
... is by definition pointless. Are you logging within that catch block? I don't think you should do that, though, because it will result in exceptions getting double-logged.
I would say you should trap exceptions at the point where it makes sense to handle them. You can make an argument for that being in ValidateUser (any exception should be 'invalid' i.e. return false), or at the top level call which might produce one (at which time you log the error). But don't catch it in both places.
You should only throw an ArgumentNullException if the argument is actually null – if you want to fail on empty too, it should be an ArgumentException. But 'guards' at the top of a method like that are fine.
|
|
|
|
|
Thanks BobJanova.
The only reason I was using "try { ... } catch { throw; }" was to make sure the original exception was passed on to the top calling method that would handle the exception and log where the exception occurred.
Regarding ValidateUser, you're right, any exception should be invalied (return false) but I know you're not mean to return anything within the catch block correct?
What would you say is best, preempting an error by having a guard at the top or letting the error happen and catching it? I know it's swings and roundabouts really but it feels that if I know the arguments might be null or empty, I should test for it first?
Is this better?:
public void ProcessPerson(Request arg)
{
try
{
if(Class2.ValidateUser(arg.Username, arg.Role)
{
}
else
{
}
}
catch(Exception ex)
{
}
}
public static bool ValidateUser(string Username, string Role)
{
using(MyClass myClass = new MyClass())
{
if(myClass.GetAdmin(Username, Role)
return true;
else
return false;
}
}
public bool GetAdmin(string Username, string Role)
{
try
{
if (String.IsNullOrWhiteSpace(UserName) || String.IsNullOrWhiteSpace(Role))
throw new ArgumentException("Username/Role cannot be empty or null");
using(MyClassAdapter myClassAdapter = new MyClassAdapter())
{
myClassAdapter.GetAdmin(Username, Role);
}
return true;
}
catch(ArgumentException ex)
{
}
catch(Exception ex)
{
}
return false;
}
If not, are you able to show me what would be best please?
Thanks a lot for your help.
modified on Wednesday, September 14, 2011 5:14 AM
|
|
|
|
|
I don't think there's anything particularly wrong with returning from a catch. The rule of thumb is that (as Pete says below) you should catch an exception where you intend to deal with it, and if you consider it dealt with you can return from the function. In this case, the calling code will not know that there was an exception in some lower layer. If you think the calling code should be informed, don't catch the exception and let the calling code deal with it.
What you've done in GetAdmin makes no sense. You're catching the exception you're throwing and doing nothing with it! Don't use exceptions to control the flow of code. If an empty or null user name/role is an error (i.e. in normal operation it should never happen), then throw the exception; if you want it to be valid input, but not an admin, then return false.
The only reason I was using "try { ... } catch { throw; }" was to make sure the original exception was passed on to the top calling method that would handle the exception and log where the exception occurred.
If you don't catch an exception, it automatically propagates up the stack. Handle it where it makes sense to do so, and once only. (Not always true, but generally you should not rethrow an exception.)
Since you included it originally I'm inclined to believe that the null or empty username is an error and GetAdmin, at least, should throw in that case. That means that, logically, it's invalid input to ValidateUser as well, I think, so I'd catch the error at the request level:
public void ProcessPerson(Request arg)
{
try
{
if(Class2.ValidateUser(arg.Username, arg.Role)
{
}
else
{
}
}
catch(Exception ex)
{
}
}
public static bool ValidateUser(string Username, string Role)
{
using(MyClass myClass = new MyClass())
{
return myClass.GetAdmin(Username, Role);
}
}
public bool GetAdmin(string Username, string Role)
{
if (String.IsNullOrWhiteSpace(UserName) || String.IsNullOrWhiteSpace(Role))
throw new ArgumentException("Username/Role cannot be empty or null");
using(MyClassAdapter myClassAdapter = new MyClassAdapter())
{
return myClassAdapter.GetAdmin(Username, Role);
}
}
It also seems like there are far too many layers involved in the authentication process. You have Class1 (which receives the request), Class2, MyClass and MyClassAdapter all involved in finding out this simple piece of information (whether a user/role combination is admin). What I typically do is at login time I populate a single UserInfo object which contains the role and user-specific access overrides, and then I can just authenticate like:
if(user != null && user.HasAccess(AccessTypes.Admin)) { ... }
(setting user to null if no user is logged in). HasAccess would look a bit like:
public bool HasAccess(AccessTypes access){
return access == access & (Access | role.Access & ~RemovedAccess);
}
I'm not sure if you can do that with enums but logically that is what AccessTypes would be, and UserInfo.Access, UserInfo.RemovedAccess and Role.Access would all be of that type. You might have to cast to int or uint to get the bit matching to work.
|
|
|
|
|
To be honest, the catch statements in your ValidateUser and GetAdmin methods are completely redundant. In general, if you find yourself using catch/throw without doing anything in there, you have poor code. You should only catch exceptions if you plan to do something with them, e.g. reattempt to acquire a transient resource that you couldn't get and which triggered the exception.
|
|
|
|
|
Thanks Pete.
Would my above post be any better? Or closer to better coding?
I'm just confused really, I think I've read too many articles on exceptions (or maybe not enough infact!). Some have said to deal with exceptions in the method that was called, others have said to let exceptions propagate to the top calling method.
(Or an alternative to the GetUser() function?)
public bool GetAdmin(string Username, string Role)
{
try
{
if (String.IsNullOrWhiteSpace(UserName) || String.IsNullOrWhiteSpace(Role))
{
LogEntry.New("Username/Role cannot be empty or null");
return false;
}
using(MyClassAdapter myClassAdapter = new MyClassAdapter())
{
myClassAdapter.GetAdmin(Username, Role);
}
return true;
}
catch(Exception ex)
{
}
return false;
}
modified on Wednesday, September 14, 2011 5:27 AM
|
|
|
|
|
OK, the simple way to think of this is to only catch an exception at the point at which you intend to do something with it. In the case you have here, you are logging the fact that something went wrong, but does it make sense to allow processing to continue? That's the question you have to ask whenever you are attempting to place exception handling in your code.
For instance, suppose you have some code to write something over TCP, and there's a failure at one point which throws an exception, do you want to attempt to reacquire a connection or abort the processing of that message at that point? The answer to that question indicates whereabouts you want to place your exception handling - if you want to reacquire, you'd handle the exception at a lower level than you would if you just wanted to abort the whole thing and tell the user that there was a problem.
|
|
|
|
|
Thanks Pete, your advice is really helpful.
Ideally when an error occurs I want to log what happened and exit. I guess I could just have one single Try/Catch block in the top calling function and catch either a custom exception and then the general Exception or maybe just the general exception? Would this be acceptable? It just doesn't seem like a clean exit, as opposed to returning false after catching an error, but that may be a misconception?
Thanks.
|
|
|
|
|
You could have all your methods contain try-catch constructs, and return a success/failure result somehow (that is what the Win32 API has done for as long as Windows exists); the disadvantage is you have to provide all the code to do that, *and* your callers have to explicitly check each and every return value.
Or you could take full advantage of the Exception mechanism, which basically says: if something goes wrong, an Exception will be thrown, and some piece of code is bound to catch it, either in the method itself, or one of the callers higher up in the calling chain; if there are no catchers, then the app will exit. You get all this for free.
So yes, a single try-catch at the top level would allow you to log the exception and exit, although that would be an ungraceful exit (and an app I would not like to use). As others have said, use a local try-catch if and when your catch block is able to resolve the problem, and leave the exception as is if you can't resolve it at that level.
And yes, input checking and throwing explicitly on bad input values is the normal thing to do.
And yes, having a catch block that just contains throw is pointless; having a catch block that contains logging and throw normally results in multiple logging. What I sometimes do is catch an exception, and throw a different exception (a "smarter" one, one that fits the usage domain of the method) while putting the original exception in the new one's InnerException. So a "File Not Found" situation deep inside a simulator program may end up as a "Simulation failed" exception, where the tree of inner exceptions ends up explaining why or how it failed.
FWIW: I would recommend you study the topic in more detail in a book on .NET in general, or on C#; an introductory book should contain a number of examples of all of the above. And I do mean a physical book, not an eBook or some web course.
|
|
|
|
|
Generally an excellent post. I'd just mention that it's good to be careful about your use of catch-throw different, because it makes debugging quite a bit harder if your IDE is set up to trip on where the exception was thrown, because you get stopped in the context of the catch handler and don't have access to the state at the point of the initial failure. It can be good but don't overdo it (as the folks who wrote the app I'm working on seem to have done).
|
|
|
|
|
Thanks Luc, your reply was really useful. I think I have a goodish idea of how to proceed (I hope). I'll make sure to get a good introductory book to give me a better understanding.
I should point out that when I said that I'd log the exception and then exit, I meant log the exception and then return a message to the user. Would that still count as an ungraceful exit if I had a single try/catch block at the top level out of interest?
Thanks
|
|
|
|
|
You're welcome.
I don't want an app to exit until I explicitly tell it to, unless it has done all it is supposed to do.
You wouldn't want MS Word to exit on a disk full situation, as that would make you loose any changes you've made to the currently open document(s).
|
|
|
|
|
using System;
using System.Collections;
public class Array
{
public static void MyArray(IEnumerable list)
{
foreach (object a in list)
{
Console.WriteLine(a);
}
}
public static void Main()
{
ArrayList array = new ArrayList(10);
array.Add("January");
array.Add("Febuary");
array.Add("March");
array.Add("April");
array.Add("May");
array.Add("June");
array.Add("July");
array.Add("Agust");
array.Add("September");
array.Add("October");
array.Add("November");
array.Add("December");
MyArray(array); Console.ReadLine();
}
}
|
|
|
|
|
Whats the question ?
When I was a coder, we worked on algorithms. Today, we memorize APIs for countless libraries — those libraries have the algorithms - Eric Allman
|
|
|
|
|
Yeah, OK, whatever, but List<string> is preferable.
|
|
|
|
|
Or, a much easier implementation that respects the current culture:
DateTime dt = new DateTime(2001, 01, 01);
for (int i = 0; i < 12; i++)
{
Console.WriteLine("Month: {0}", dt.AddMonths(i).ToString("MMMM"));
}
|
|
|
|
|
I created a SnapIn Page for IIS. Its displayed as a tab, when i right click on any of the virtual directories in IIS Manager.
Its called MySettings. Its shown along with other tabs like virtual directory, Documents, Directory Security and so on.
But when trying to view properties of any of the virtual directory IIS Crashes.
The IIS Manager window simply vanishes. (mmc.exe crashes)
I debugged my SnapIn C# code. But it does not even break in initialization of SnapIn Control.
So why does in the first place it crashes, as it does not yet loaded my Page????
Crash is temperamental sometimes it works and sometimes it crashes ?????
But on removing the page from MMC SnapIn List by deleting the registry entry for my Page located at.
HKLM\Software\Microsoft\MMC\SnapIns, IIS manager works perfectly. Now it no longer crashes.
Used debuggers like DrWatson and Debug Diag Tool but could not get any informative data.
Please, anyone can share with me ideas as to how this could be tackled?
Thanks in advance.
Regards.
Pratik.
|
|
|
|
|
private void button1_Click(object sender, EventArgs e)
{
string[] input = new string[] { "Web", "Images", "Gmail", "C:\\Users\\Amit_Personal\\Documents\\Visual Studio 2008\\Projects\\abc\\abc\\google_image_final.png", "Google Search", "I'm Lucky"};
// Pass the array as a parameter:
Designpage(input);
}
public void Designpage(string[] arr)
{
string fileLoc = @"C:\Users\Amit_Personal\Documents\Visual Studio 2008\Projects\abc\abc\a.htm";
if (File.Exists(fileLoc))
{
using (StreamWriter s = File.AppendText(fileLoc))
{
// StringWriter s = new StringWriter();
s.WriteLine("{0}", "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.0 Transitional//EN\">");
s.WriteLine("{0}", "<html>");
s.WriteLine("{0}", "<title>Google</title>");
s.WriteLine("{0}", "<link rel=\"stylesheet\" type=\"text/css\" href=\"style.css\">");
s.WriteLine("{0}", "</head>");
s.WriteLine("{0}", "<body>");
s.WriteLine("<h4>{0} ", arr[0]);
s.Write("{0} ", arr[1]);
s.WriteLine("{0} </h4>", arr[2]);
s.WriteLine("<img src=\"{0}\" />",arr[3]);
s.WriteLine("{0}", "</body>");
s.WriteLine("{0}", "</html>");
s.WriteLine("{0}", "");
s.Close();
}
}
Any one Know how to write better code to generate a .html file using C# language..
|
|
|
|
|
using (StreamWriter sw = File.AppendText(fileLoc))
{
sw.WriteLine(string.Format(@"
<html>
<title>Google</title>
</head>
<body>
{0}
{1}
{2}
<img src=\"{3}\" />
</body>
</html>", arr[0], arr[1], arr[2], arr[3]);
}
Bastard Programmer from Hell 
|
|
|
|
|
You could create a file that is the template and contains the content of the markup. for example:
<html>
<body>
<div> blah blah blah... </div>
<h1>{0}</h1>
<p>
{1}<br />
<a href="{2}">{3}</a>
</p>
</body>
</html>
Then you can read in that file and use that in a single string.Format line to create the files from a template.
public void DesignPage(string TemplateFile, string OutputFile, string[] Contents){
string content = File.ReadAllText(TemplateFile);
content = string.Format(content, Contents);
File.WriteAllText(OutputFile, content);
}
You can also update the file templates without having to recompile, etc.
|
|
|
|
|
Here's one other way to do it... Not necessarily the best, but different, and prevents you from forgetting a closing tag.
XDocument doc = new XDocument(
new XDocumentType("HTML", "PUBLIC", "-//W3C//DTD HTML 4.0 Transitional//EN"),
new XElement("html",
new XElement("head",
new XElement("title", "Google"),
new XElement("link",
new XAttribute("rel", "stylesheet"),
new XAttribute("type", "text/css")
)
),
new XElement("body",
new XElement("h4", arr[0] + arr[1] + arr[2]),
new XElement("img",
new XAttribute("src", arr[3])
)
)
)
);
Or something like that...
|
|
|
|
|
Similar to what Ian said, but using an XmlDocument:
(Strictly off-the-cuff, not tested, however I did something like this last week to generate an HTML Table in a string)
XmlDocument doc = new XmlDocument() ;
XmlNode nod ;
doc.AppendChild ( doc.CreateElement ( "html" ) ) ;
doc.DocumentElement.AppendChild ( nod = doc.CreateElement ( "head" ) ) ;
nod.AppendChild ( nod = doc.CreateElement ( "title" ) ) ;
nod.InnerText = "Google" ;
doc.DocumentElement.AppendChild ( nod = doc.CreateElement ( "body" ) ) ;
...
doc.WriteTo ( somestream ) ; // To write to a stream
somestring = doc.OuterXml ; // To copy to a string
Also,
amit_ghosh18 wrote: s.WriteLine("{0}", "<html>");
you needn't use the "{0}" to write out a single string:
s.WriteLine ( "html" ) ;
s.WriteLine ( somestring ) ;
|
|
|
|
|
Why are you trying to make a fake Google site?
For anything but the most trivial example I second the idea of using a template, and substituting special tokens for items in your data. In my HTTP server[^] the SubstitutingFileReader uses special tokens that look like HTML tags, which makes sense if the template is HTML (or another SGML language).
|
|
|
|
|
I am wondering if you can tell me what would be the good items I can drag and drop from the visual studio asp.net 2010 tool kit that would acomplish the following tasks:
1. most of the data would be contained in a datagrid,
2. I would like to have a tab on the top of the webpage where the user can tab to over click the 'next' button to accomplish the following tasks:
*The first tab will:
a. allow the user to have a 'type' ahead feature so they can select what organization they want to work with,
b. there will be a datagrid where the user can check off what suborganizations that are affect,
c. There will be a datepicker so the user can select certain dates, and d. there would be a 'next' button and/or tab on top for the user to go to the next page,
e. On this page, there will be a 'save draft' button. This will not commit the data to the database, but it will save the work entered so far.
f. On this page there will be a restart button. Basically this clear out any data and allow the user to start from the beginning again
*The second tab will:
a. display the data entered on the previous page on the top.
b. there will be alot of rows where the data will be prepopulated, and/or allow the user to enter data.
c. This page will show any errors that the user made.
d. This page will also contain a description of what what kind of data is entered on that row,
e. This page will have a button or tab to update the database.
f. There will be a 'save draft' button. This will not commit the data to the database, but it will save the work entered so far.
g. On this page there will be a restart button. Basically this clear out any data and allow the user to start from the beginning again.
h. The final tab will show summary information of what was just entered.
|
|
|
|
|
|
We don't do homework for you. What possibly could you learn from just copying and pasting an answer from some caring CPians response and turning it in. If that is how you want to get your marks you will seriously need to reconsider your career expectations and brush up on your fries into oil dipping skills because that is what your skills will be worth if you go down this road.
I wasn't, now I am, then I won't be anymore.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
