Interfaces vs Abstract Classes

[Hamster]

Pass the buck design pattern

 

[Solarion]

Replace it with "throw;" (i.e. remove the ex) so you at least keep the original stack trace. "throw ex;" resets it. Won't make much of a difference here but now you know.

Thanks!

 

[Spacerat]

Omg this could be on a t-shirt. Aaahaha.

It's just for testing really, it gives me the most direct answer to what is going on. I'll clean it up later.

No, clean it up now. There is no later. You are knowingly incurring technical debt. Even if you catch it at a higher level, you will still see the full exception and stack trace;

Don't catch an exception if you are not adding value. Adding value means logging the exception, translating to a user friendly msg etc etc. Or rolling back the transaction. Otherwise don't. Let it bubble up where you actually want to do something useful with it.

E.g.

StartTransaction();
try
{
DbStuff();
CommitTransaction();
}
catch
{
RollbackTransaction();
throw; // It is cheaper to rethrow than to throw a new exception.
}

 

[Solarion]

No, clean it up now. There is no later. You are knowingly incurring technical debt. Even if you catch it at a higher level, you will still see the full exception and stack trace;

Don't catch an exception if you are not adding value. Adding value means logging the exception, translating to a user friendly msg etc etc. Or rolling back the transaction. Otherwise don't. Let it bubble up where you actually want to do something useful with it.

E.g.

StartTransaction();
try
{
DbStuff();
CommitTransaction();
}
catch
{
RollbackTransaction();
throw; // It is cheaper to rethrow than to throw a new exception.
}

Better?

 

[_kabal_]

You need to "try" after creating the transaction. This will enable you to avoid "if tx!= null then tx.Rollback()" in you catch block.

As it stands now you could throw a null pointer exception in you "catch" block. This could happen if for instance "conn.Open()" throws an exception.

 

[Solarion]

You need to "try" after creating the transaction. This will enable you to avoid "if tx!= null then tx.Rollback()" in you catch block.

As it stands now you could throw a null pointer exception in you "catch" block. This could happen if for instance "conn.Open()" throws an exception.

Brain overload for me. I'm lost.

 

[Pho3nix]

Brain overload for me. I'm lost.

Because your transaction "tx" is initialized to null and not an actual transaction. If you have an exception before the conn.beginTransaction() you'll get another exception since the transaction "tx" is null.

Hope that made sense.

 

[Solarion]

Because your transaction "tx" is initialized to null and not an actual transaction. If you have an exception before the conn.beginTransaction() you'll get another exception since the transaction "tx" is null.

Hope that made sense.

Oh yeah that does now thanks I see the problem. I'll rework it.

 

[Solarion]

This looks about the most straight forward way using transactions.

https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqltransaction.commit(v=vs.110).aspx

protected void executeInsertQuery(String _query, SqlParameter[] sqlParameter)
        {
            using (SqlConnection connection = new SqlConnection(base.ConnectionString))
            {
                connection.Open();

                SqlCommand command = connection.CreateCommand();
                SqlTransaction transaction;

                transaction = connection.BeginTransaction();

                command.Connection = connection;
                command.Transaction = transaction;

                if (sqlParameter != null)
                {
                    command.Parameters.AddRange(sqlParameter);
                }

                try
                {
                    command.CommandType = CommandType.StoredProcedure;
                    command.CommandText = _query;
                    command.ExecuteNonQuery();

                    transaction.Commit();

                }
                catch (Exception ex)
                {
                    Console.WriteLine("Commit Exception Type: {0}", ex.GetType());
                    Console.WriteLine("  Message: {0}", ex.Message);

                    try
                    {
                        transaction.Rollback();
                    }
                    catch (Exception ex2)
                    {
                        Console.WriteLine("Rollback Exception Type: {0}", ex2.GetType());
                        Console.WriteLine("  Message: {0}", ex2.Message);
                    }
                }
            } 
        }

 

[Spacerat]

This looks about the most straight forward way using transactions.

https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqltransaction.commit(v=vs.110).aspx

protected void executeInsertQuery(String _query, SqlParameter[] sqlParameter)
        {
            using (SqlConnection connection = new SqlConnection(base.ConnectionString))
            {
                connection.Open();

                SqlCommand command = connection.CreateCommand();
                SqlTransaction transaction;

                transaction = connection.BeginTransaction();

                command.Connection = connection;
                command.Transaction = transaction;

                if (sqlParameter != null)
                {
                    command.Parameters.AddRange(sqlParameter);
                }

                try
                {
                    command.CommandType = CommandType.StoredProcedure;
                    command.CommandText = _query;
                    command.ExecuteNonQuery();

                    transaction.Commit();

                }
                catch (Exception ex)
                {
                    Console.WriteLine("Commit Exception Type: {0}", ex.GetType());
                    Console.WriteLine("  Message: {0}", ex.Message);

                    try
                    {
                        transaction.Rollback();
                    }
                    catch (Exception ex2)
                    {
                        Console.WriteLine("Rollback Exception Type: {0}", ex2.GetType());
                        Console.WriteLine("  Message: {0}", ex2.Message);
                    }
                }
            } 
        }

Why the nested try? Don't need it... Also you need to rethrow the original..

try
{
your stuff
}
catch
{
tx.rollback();
throw;
}

 

[[)roi(]]

Never much liked the try {} catch {} approach; an either monad with bind is always more elegant.

 

[C4Cat]

Why the nested try? Don't need it... Also you need to rethrow the original..

try
{
your stuff
}
catch
{
tx.rollback();
throw;
}

Why do you need to throw the exception?

 

[etienne_marais]

By this time Solarion does not know where to hold and where to let go. I think Spacerat is wrong, but if he is not then I'm confused as well.

 

[Solarion]

By this time Solarion does not know where to hold and where to let go. I think Spacerat is wrong, but if he is not then I'm confused as well.

I took it directly from Microsoft. If Spacerat is right then we are all in some trouble.

But yes at this point I'm completely lost.

 

[[)roi(]]

Be careful not to wrap all neighbouring code in the function with a try {} catch {} block, as this could easily hide errors; and similarly be cautious with re-throws of exceptions up the stack; with the only exception being closures to an outer scope.

Note: You can't avoid try {} catch {} for functions that use them; however you can wrap these, in a similar fashion to the C Interop.

 

[Hamster]

You don't need to throw again if you've handled the exception. Rethrowing it to control flow is wrong and you should rather be returning an error code or false or something.

The only real reason you'll "throw;" a caught exception is if you're debugging and intending to remove the try catch.

My opinion anyway

 

[Solarion]

Keeping it simple then. Have added a logger.

SqlTransaction transaction;

   try
   {
        command.CommandType = CommandType.StoredProcedure;
        command.CommandText = _query;
        command.ExecuteNonQuery();

        transaction.Commit();

    }
    catch (Exception ex)
    {
        transaction.Rollback();
        LoggError.Write(ex);
    }

public static class LoggError
    {
        public static void Write(Exception exception)
        {
            string logFile = String.Empty;
            StreamWriter logWriter;
            try
            {
                logFile = ConfigurationManager.AppSettings["ErrorLog"].ToString();
                if (File.Exists(logFile))
                    logWriter = File.AppendText(logFile);
                else
                    logWriter = File.CreateText(logFile);
                logWriter.WriteLine("=>" + DateTime.Now + " " + " An Error occured : " +
                    exception.StackTrace + " Message : " + exception.Message + "\n\n");
                logWriter.Close();
                throw exception;

            }
            catch (Exception e)
            {
                throw;
            }
            finally
            {

            }
        }
    }

 

[[)roi(]]

@solarion; simple is always the goal, however loggers doesn't quite cover what's IMO missing from your code examples i.e. explicit management of state; the functions you've shared recently all e.g. return void; so how exactly are you keeping track of what's worked and what's failed.

It's fairly rare that functions are completely independent entities; and usually form a link in a process chain. Re-throwing the exceptions up the stack would be 1 way to track this, another would be through return values.

 

[Solarion]

@solarion; simple is always the goal, however loggers doesn't quite cover what's IMO missing from your code examples i.e. explicit management of state; the functions you've shared recently all e.g. return void; so how exactly are you keeping track of what's worked and what's failed.

It's fairly rare that functions are completely independent entities; and usually form a link in a process chain. Re-throwing the exceptions up the stack would be 1 way to track this, another would be through return values.

I'm getting there. At the moment is just crashes and burns. Error handling is an area I'm sorely lacking on. Newbie coder remember so at the moment I'm actually doing this to learn and when I fudge it up, I just keep at it until I get it right.

 

[[)roi(]]

I'm getting there. At the moment is just crashes and burns. Error handling is an area I'm sorely lacking on. Newbie coder remember so at the moment I'm actually doing this to learn and when I fudge it up, I just keep at it until I get it right.

Sure...it's YOLO scenario:

Btw, my comments (and that of my MyBB peers) aren't meant to criticise, rather to share experience or to encourage you to consider alternatives.

Never much liked the try {} catch {} approach; an either monad with bind is always more elegant.

I referred earlier to a "either monad with bind"; which as a basis deals with outcomes, for example:

• Success
• ...or, Failure

...depending on the outcome, you could either continue the process chain or terminate (i.e. for recourse at the point of initiation).

Bind in short ties up the input and outputs i.e. 1 function feeds into another in a chain of events. If any link (function) fails; the link is terminated; however the cause is always captured and relayed back to initiating event.

For example:

var a = function1(arg1); 
var b = function2(a)
....
var z = function27(y)

We call function1and store the result in "a"; and then call function2 and pass in the function 1 result: "a". "a" could be as example:

• a reference to an instance,
• value
• an error code.

Either way if function2 is dependent on the result of function1; "a" should ideally be sufficient for function2 to determine is it can safely proceed or whether it needs to abort and relay the failure back to the originating event.

Naturally all of this can be expressed i.t.o. if-elseif-else statements.

Bind FYI is a similar approach to the if-elseif-else one, except that it follows a more declarative and terse style. The Either type btw is just an approach to encapsulate two possible outcomes, e.g. Success or Failure. Success would be what the next function in the call chain would require, whereas Failure would be an instruction to that function to bypass the processing in order to return the failure up the call stack.

Similarly you'll find code that uses exceptions for this purpose; but in practice that can easily lead to more try {} catch {} wrappers that ultimately just ignore the underlying fragility. Worse you have an implicit result i.e. your function shows it returns nothing; void (an Action), however a re-throw is also a result, except it's anything but explicit.

Using a Either style monad for the result type would mean your functions are modelled more like this:

private static Result<Success, Failure> function1(InputType input) { }
private static Result<Success, Failure> function2(Result<Success, Failure> input) { }

// this is called as follows

var a = function1(inputValue)
               .bind(function2)
               .bind(function3)
               .bind(function4);

We start with function1, and bind into function2 with the result from function1. function2 then has internal code to explicitly deal with deal with Success or Failure; Failure is a simple bypass, simply returning the input (i.e. the 1st issue), ultimately leading to that Failure being returned up the call stack to initiating call: "a" would either be the result of a Success or Failure.

Let me know if you'd like to see how to implement this in C#. Not that much code is required.