Customized Exception

I am building my "base exception to end all base exceptions" class in C# and was looking for some opinions on my implementations so far. Ideally my usage of integrated Rethrow, Throw, and ThrowFatal methods for some syntactic sugar.

```
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Schloss
{
[Serializable]
public class SchlossException : Exception
{
// member variables
private bool _fatal = false;
private bool _rethrown = false;
private ArrayList _additionalInfo = new ArrayList();

public bool Fatal { get { return _fatal; } }
public bool Rethrown { get { return _rethrown; } }
public ArrayList AdditionalInfo { get { return _additionalInfo; } }

public SchlossException() { }
public SchlossException(string message) : base(message) { }
public SchlossException(string message, Exception inner) : base(message, inner) { }
protected SchlossException(
System.Runtime.Serialization.SerializationInfo info,
System.Runtime.Serialization.StreamingContext context)
: base(info, context) { }

// rethrow an exception to preserve the call stack across threads
public void Rethrow()
{
_rethrown = true;

this.Throw();
}

// throw this sexception
public void Throw()
{
throw this;
}

// throw with the fatal flag, in case anyone is listening o_O
public void ThrowFatal()
{
_fatal = true;

this.Throw();
}

// poopy, messy code
public SchlossException(string Msg, params object[] AddInfLst)
: base(Msg, SchlossException.GetInnerException(AddInfLst))
{
this.AppendAdditionalInfo(AddInfLst);
}

private static Exception GetInnerException(params objec

You're using ArrayList which is the weakly typed version of List. Now, since apparently this is done because you're using object[], I would suggest to just strongly type this to List 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 +=.

https://stackoverflow.com/questions/4191079/does-stringbuilder-use-more-memory-than-string-concatenation

StackExchange Code Review Q#78595, answer score: 12