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 likeExceptionHelpers
-- notSchlossException.GetInnerException()
.Is the first entry always the
Exception
? Apparently not because you useas
which indicates that it may not be. What is the expected behaviour when index1
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 +=
.