You're using ArrayList
which is the weakly typed version of List<T>
. Now, since apparently this is done because you're using object[]
, I would suggest to just strongly type this to List<object>
instead.
This is assuming your additional information is in fact different unrelated types and not always a string
, for example.
You're explicitly naming the System.Runtime.Serialization
namespace instead of using a using
statement. I don't immediately see any naming collisions so I would omit this verbosity.
this.Throw()
is more verbose than it should be: you don't have to make any distinction between Throw()
methods so you might as well call Throw()
.
Since Throw()
only calls throw this;
, I might be tempted to just omit the method altogether since you're adding another layer of complexity for really just two words.
Some comments should probably be rephrased in a professional environment as I'm sure you're aware.
Group constructors so they're all in once place.
Parameters are written in lowerCamelCase, so Method(string Msg)
becomes Method(string message)
.
Don't abbreviate words. Msg
becomes message
, AddInfLst
becomes additionalInfo
.
There's no point in saying something is a list if you can look at the type and see it's.. an array? That illustrates the problem with this perfectly: your variable's name does not take changes to its type in account. It doesn't add any information that I didn't know from the type; in fact it even added confusion.
private static Exception GetInnerException(params object[] AddInfLst)
{
Exception Exc = null;
if (AddInfLst.Length > 1)
{
Exc = (AddInfLst[0] as Exception);
}
return Exc;
}
This is a curious piece of code. Let me sum up my frowns:
Should this even be in this class? Why not a utility class? If I needed to get the innerexception from an object[]
, I'd look for something like ExceptionHelpers
-- not SchlossException.GetInnerException()
.
Is the first entry always the Exception
? Apparently not because you use as
which indicates that it may not be. What is the expected behaviour when index 1
has the exception?
No documentation that explains how this method would work.
if (AddInfLst == null)
{
return;
}
You don't need this statement since passing no arguments to a params
method will result in an empty array. Therefore it will evaluate the loop but never enter it, nor throw a NullReferenceException
on AddInfLst.Length
.
Local variables use lowerCamelCase so Obj
becomes obj
.
It is better to follow this idiom:
var x = y as z;
if(x != null)
{
// use x
} else {
// use y
}
rather than this:
if(y is z)
{
var x = y as z;
} else {
// use y
}
It's a matter of 2 casts vs 1.
Create an empty string using string.Empty
, it makes the intent more clear than ""
.
When you're looping, use a StringBuilder
to concatenate strings instead of +=
.
http://stackoverflow.com/questions/4191079/does-stringbuilder-use-more-memory-than-string-concatenation