Interfaces vs Abstract Classes

Hamster said:
Well this thread has gone off topic...
Click to expand...

^ Thank you.

Can OP tell us if he grasps the concept for catching the exception to do the rollback and then having to rethrow (or not). It is also considered bad practice to throw new Exception(); You should be throwing different types of exception depending on the type of problem. In order for you to distinguish in the catch and handle them differently:

try
{
}
catch (FileNotFoundException fexc)
{
// handle the case where the file is not found
}
catch (ArgumentException aexc)
{
// handle the case where an argument was invalid
}
|
|
catch (Exception exc)
{
// handle the general case where the exception is not any of the above
}

The .NET framework documentation tells you for each method what type of exception will be raised in different scenarios.
 
I've gone through all the posts since my last one and busy chewing them over. I just wanting to add to what you just posted Spacerat.

In my understand, if, say for example which is one of the errors I have been simulating, I stop SQL Server, then run the app. This can be a common problem if say the network is down. I then run my application and get this, and that's about as far as the that mule is willing to walk, he won't go any further and just sits there like that. I am logging the exceptions now so there's progress.

The thing is, that popup is stating the obvious at this point: SqlException was unhandled.

Capture.jpg
 
Last edited:
You know what the scary thing is, I have about 200+ projects I've downloaded from codeproject over the past year or so. None and I mean NONE of them have sufficient error handling. They simply catch the exception, throw ex, and leave it at that.

I think a lot of developers think "Aah **** it I'll leave it to the end user to figure out and swear at."

Edit:

This is from the book Pro CSharp 5.0 and the .NET Framework by Andrew Troelsen

PHP: 
public void ProcessCreditRisk(bool throwEx, int custID)
        {
            // First, look up current name based on customer ID.
            string fName = string.Empty;
            string lName = string.Empty;
            SqlCommand cmdSelect = new SqlCommand(
            string.Format("Select * from Customers where CustID = {0}", custID), sqlCn);
            using (SqlDataReader dr = cmdSelect.ExecuteReader())
            {
                if (dr.HasRows)
                {
                    dr.Read();
                    fName = (string)dr["FirstName"];
                    lName = (string)dr["LastName"];
                }
                else
                    return;
            }
            // Create command objects that represent each step of the operation.
            SqlCommand cmdRemove = new SqlCommand(
            string.Format("Delete from Customers where CustID = {0}", custID), sqlCn);
            SqlCommand cmdInsert = new SqlCommand(string.Format("Insert Into CreditRisks" +
            "(CustID, FirstName, LastName) Values" +
            "({0}, '{1}', '{2}')", custID, fName, lName), sqlCn);
            // You will get this from the connection object.
            SqlTransaction tx = null;
            try
            {
                tx = sqlCn.BeginTransaction();
                // Enlist the commands into this transaction.
                cmdInsert.Transaction = tx;
                cmdRemove.Transaction = tx;
                // Execute the commands.
                cmdInsert.ExecuteNonQuery();
                cmdRemove.ExecuteNonQuery();
                // Simulate error.
                if (throwEx)
                {
                    throw new Exception("Sorry! Database error! Tx failed...");
                }
                // Commit it!
                tx.Commit();
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex.Message);
                // Any error will roll back transaction.
                tx.Rollback();
            }
        }

And this is from another highly recommended book called Beginning C# Object-Orientated Programming:

PHP: 
public ArrayList ListNames()
        {
            string connString = "Data Source=LocalHost;Initial Catalog=pubs;Integrated Security=True";
            using (SqlConnection pubConnection = new SqlConnection(connString))
            {
                using (SqlCommand pubCommand = new SqlCommand())
                {
                    try
                    {
                        pubConnection.ConnectionString = connString;
                        pubConnection.Open();
                        pubCommand.Connection = pubConnection;
                        pubCommand.CommandText =
                        "Select lname from employee";
                        using (SqlDataReader employeeDataReader = pubCommand.ExecuteReader())
                        {
                            ArrayList nameArray = new ArrayList();
                            while (employeeDataReader.Read())
                            {
                                nameArray.Add(employeeDataReader["lname"]);
                            }
                            return nameArray;
                        }
                    }
                    catch (SqlException ex)
                    {
                        throw ex;
                    }
                }
            }
        }

As you can see, there is not a lot of love for the handling of Exceptions.

Edit: This advice from a poster on CodeProject
It's generally accepted as bad practice to throw exceptions for controlling program flow. You've already detected it in your code so it's not an 'exceptional' circumstance i.e. you can deal with it immediately without introducing the overhead of an exception..(which you appear to be immediately catching in the same function.)

Also 'throw ex' - this is such bad advice to junior programmers. If you used 'throw' rather than 'throw ex' you would be preserving the call stack information.

catch (Exception exception)
{
throw ;
}
Click to expand...

Original article
 
Last edited:
Unless you're developing a reusable library you should just forget the throw keyword exists.

Once you've caught an exception you know something has gone wrong and are in control. Handle it (log it, cleanup resources etc.) and return an appropriate indicator that something has gone wrong (like a false).

The calling code then knows "hey, that record didn't insert properly, let me tell the UI" and a message is displayed "Hey, bro, something bad happened. Please try again or contact your admin if it persists."

But rethrowing an error means you are out of control and hoping something else will catch it. You are now dictating program flow with these exceptions and that's just bad practice. Lots of others issues as well like speed: exceptions are slow.

So until you know what you are doing, know that there is something like throw but never ever use it.
 
Last edited:
Solarion said:
You know what the scary thing is, I have about 200+ projects I've downloaded from codeproject over the past year or so. None and I mean NONE of them have sufficient error handling. They simply catch the exception, throw ex, and leave it at that.

I think a lot of developers think "Aah **** it I'll leave it to the end user to figure out and swear at."
Click to expand...
Bad implementation is certainly not limited to this; the underlying issues tend to repeat. e.g. those who struggle with exceptions, tend to struggle with testing, similarly partial vs. total functions, ... (it's a long list)

Hamster said:
Unless you're developing a reusable library you should just forget the throw keyword exists.

Once you've caught an exception you know something has gone wrong and are in control. Handle it (log it, cleanup resources etc.) and return an appropriate indicator that something has gone wrong (like a false).

The calling code then knows "hey, that record didn't insert properly, let me tell the UI" and a message is displayed "Hey, bro, something bad happened. Please try again or contact you're admin officer it persists."

But rethrowing an error means you are out of control and hoping something else will catch it. You are now dictating program flow with these exceptions and that's just bad practice. Lots of others issues as well like speed: exceptions are slow.

So until you know what you are doing, know that there is something like throw but never ever use it.
Click to expand...
Good advice, however I'd even argue that far too much code and too many reusable libraries (incl. .NET) abuse exceptions, because like nulls it's the easy (lazy) way out.

@etienne_marais coined the phrase "Pass the buck design pattern" -- remember there isn't just one way to do this. There are however many bad practices; so try to identify these (@Hamster hit on a few) and then explicitly avoid them.

A simple litmus test for a good function and approach; is if its easy to test -- specifically that it doesn't involve complex "set-up the world" initialisation.
Ps. Partial functions, void functions, hidden input / output and exceptions are rarely easy to test.
 
Last edited:
Copy/Paste programmers are bad irrespective of language or paradigm; and if that's what an education platform is delivering then question the quality of the tutoring.
Click to expand...
One of these sits in the next cubicle here.... :rolleyes:

She is currently 1 month over due date. The deadline went whooshing by... Maar nou ja, ons is meer ge-worry oor die f*kken klok masjien en werks-ure en hoe lank mense op die toilet sit in die dag.
 
So I remembered I didn't actually mention what I am doing...

Don't laugh- yes, this little squirrel is fixing their f**kups.
Here is the story:

So a leading bank wants to kick this outfit to the kerb, because, transactions take waaaaay too long to be processed. So you can see where I am going with this right?

The code is in C, which is to be expected, it is, after all, an embedded system at the end of the day.
Unfortunately, the people who have worked on it in the past, are all "graduates", so they have basically taken already bad code from the vendor (vendors don't care in this game) and mashed that up even worse. Its basically them trying to make C do C++, and not knowing that there is no garbage collection, no OS, and certainly no object orientation.

So basically it has grown, to around 120 source files, some of which serve no purpose, and in many cases they didn't even know that header files are required, so when I got here, they would copypasta from some mainline version of a project, and modify that to taste. And because they have no clue about how the linker script works, or the fact that this is sde-gcc doing the work underneath the Eclipse bling bling, the stuff has to be copied to the exact correct path on a hard disk, or it doesn't compile.

So what did the Squirrel do?

I fixed a lot of this tripe, because it drove me crazy.

Firstly I fixed the linker script, wrote a new one, so now... OMG, the project can actually be checked out of subversion (another thing I had to build when I got here) and it can be copied anywhere the SDE-GCC compiler just works fine.

Secondly I spent two weeks fixing up all the missing header file issues... From 600+ compile time warnings we are now down to 57.

So good, that is all done, and I call a spade a spade, told them their code is shyte. So time comes to implement a small project.. Guess what, I run into issues with communications, takes too bloody long, and it is hard to get working. So yes... I get given permission to rewrite some of the code...

OMFG... the code I saw in there.... ladies and gentlemen.. when I say its for the books, its for the books... whomever wrote that crap has no clue and should not be allowed near a computer.

So it has taken me 2 1/2 weeks to rewrite an ENTIRE communications library, and because these idiots don't know that in a file, they neglected to use, there are static symbols that you can use to access the circular buffer in hardware, they used a function somewhere to read socket data, byte at a time.

So now, we read an entire PACKET at a time, and guess what else, I have implemented the standard C function read() to read stuff from the socket... and hazard a guess at the speed this all takes place... 10-15mS at most.. basically now the terminal is up against the speed of the GPRS/3G network. Opens a socket to my test Amazon AWS instance in less than 1s...

And of course, by boss just doesn't understand a bloody thing I have done... all he is interested in is seeing stuff on the LCD display...

:whistle: These guys don't know anything about OSS or Linux, or even what a class/object really is.
 
Solarion said:
I've gone through all the posts since my last one and busy chewing them over. I just wanting to add to what you just posted Spacerat.

In my understand, if, say for example which is one of the errors I have been simulating, I stop SQL Server, then run the app. This can be a common problem if say the network is down. I then run my application and get this, and that's about as far as the that mule is willing to walk, he won't go any further and just sits there like that. I am logging the exceptions now so there's progress.

The thing is, that popup is stating the obvious at this point: SqlException was unhandled.

View attachment 412876
Click to expand...


What this means is that your code threw an exception. You handled it by logging and rethrowing., But you did not handle the rethrown exception at a higher level. The IDE catches it and sits there. If you wrap the call to this method with try-catch, you will see the code entering that catch...

Also there is a diff between throw ex; and throw; when you rethrow the exception.
http://stackoverflow.com/questions/730250/is-there-a-difference-between-throw-and-throw-ex
 
Solarion said:
You know what the scary thing is, I have about 200+ projects I've downloaded from codeproject over the past year or so. None and I mean NONE of them have sufficient error handling. They simply catch the exception, throw ex, and leave it at that.
Click to expand...

Very very true. But then the articles and projects on CodePorject are usually just concepts to show an idea and hardly fully-fledged, robust projects. But having said that, very few devs I have encountered have a well-thought out and implemented error handling strategy. I often wonder what happens to such a system when a failure happens. Crash? Does it corrupt data?

Hamster said:
Unless you're developing a reusable library you should just forget the throw keyword exists.

Once you've caught an exception you know something has gone wrong and are in control. Handle it (log it, cleanup resources etc.) and return an appropriate indicator that something has gone wrong (like a false).

The calling code then knows "hey, that record didn't insert properly, let me tell the UI" and a message is displayed "Hey, bro, something bad happened. Please try again or contact your admin if it persists."

But rethrowing an error means you are out of control and hoping something else will catch it. You are now dictating program flow with these exceptions and that's just bad practice. Lots of others issues as well like speed: exceptions are slow.
Click to expand...

Agreed. You ONLY catch the exception if you are going to do something useful at that level. Things like disposing resources and rollbacks if required. Otherwise, DON'T!!

And then ONLY when you catch at that lower level, you need to rethrow if you want the exception to bubble further up the call stack.

Personally, in Solarion's example, I would NOT catch the exception for the purposes of logging at that level. Reason is that you are making your DAL dependent on the (static) Error logger. This is called an Ambient Context. It means that where-ever you use your DAL, you need to include THAT PARTICULAR IMPLEMENTATION of the error logger. Not very extensible.

My preferred option would be to let the exception just bubble up to a higher level, e.g. your BLL and then log there.

Whether you want to log at this low level or higher level, consider rather ctor-injecting an ILogger instance. This enables you to plug in any logging mechanism. File/Console/HTTP Post to JIRA/etc etc. Or even an ILogger implentation that logs to multiple loggers, e.g depending on severity.

SO if we have:

Code: 
public interface ILogger
{
  void Log(string msg,int severity);
}

Then your logger implementations could look like:
Code: 
public class FileLogger:
  ILogger
  {
    public void Log(string msg,int severity)
    {
      // Write To File
    }
   }

public class ConsoleLogger:
  ILogger
{
  public void Log(string msg,int severity)
  {
    // Write To Console
  }
}

public class JiraLogger:
  ILogger
{
  public void Log(string msg,int severity)
  {
    // Post to JIRA  
  }
}

The class below uses Composition to reuse other classes that you don't have to touch (because they work). The 'O' in SOLID (Open/Closed Principle). Composition is a very powerful way to reuse code. This composite logger still implements the ILogger interface.

Code: 
public class MultipleLogger:
  ILogger
{
  public MultipleLogger(ILogger consoleLogger,ILogger fileLogger,ILogger jireLogger)
  {
    this.consoleLogger = consoleLogger;
    this.fileLogger = fileLogger;
    this.jiraLogger = jiraLogger;
  }

  public void Log(string msg,int severity)
  {
     consoleLogger.Log(msg,severity);
     fileLogger.Log(msg,severity);

     if (severity > 10)
       jiraLogger.Log(msg,severity);
  }

private readonly ILogger consoleLogger;
private readonly ILogger fileLogger;
private readonly ILogger jiraLogger;
}

Then in your BLL or DAL:

Code: 
public class MyBll
{
  public MyBll(ILogger logger)
  {
    this.logger = logger;  //  <---- Contructor injection (DI)
  }

  public void Method()
  {
     try
     {
        SomeMethodThatCanThrowException();
     }
     catch (Exception exc)
     {
        logger.Log(SomeMessageOrExceptionMesaage,2);
     }
  }

  private readonly ILogger logger;
}

When you instatiate your BLL you can inject any logger into it.

Code: 
ILogger logger = new ConsoleLogger();

MyBll b = new MyBll(logger);

or


Code: 
ILogger logger = new FileLogger();

MyBll b = new MyBll(logger);

The BLL is none the wiser as it only needs to know how to use the ILogger service. The BLL does not care where it goes to. Often, you also want to not log at all. You dont want to remove all logging code. Simply implement a NullLogger:


Code: 
public class NullLogger:
  ILogger
{
  public void Log(string msg,int severity)
  {
    // Do nothing
  }
}

and ctor-inject

Code: 
ILogger logger = new NullLogger();

MyBll b = new MyBll(logger);

The above example abstracts the BLL from the logger implementation. Think this is what the thread was about originally. The BLL does not care or know what the implementation is. You can substitute any implementation as long as it adheres to the ILogger interface contract. This is the 'L' in SOLID. Liskov's Substitution Principle.

When using the DAL in the example as a supplied DLL assembly, the supplier of the assembly has to also supply the assembly that contains that particular implementation of ErrorLogger because the DAL has a dependency on that error logger implementation. Dependencies on specific implementations is bad. The assembly supplier also has to tell the dev that it needs to be referenced in the project. Apart from that the dev will never know up until he gets a compile error. Also, the dev then has to accept that particular way of logging used by that code.

By having the ILogger interface as an argument in the BLL ctor, the BLL class advertises the need to have an ILogger service injected. You cannot instantiate the class without it. Therefore the dev knows what the BLL class needs to function correctly AND he has the option to plug in a ILogger implementation that is compatible with/same as the logging mechanism of the rest of the application. This is how you improve extensibilty.
 
Last edited:
Randy_The_Squirrel said:
Absolute beauty in code...
Click to expand...

Yeah, DI is a beauty until this inevitably happens...

Code: 
class FIXAdapter(ILog log, IUserRepo userRepo, IPortfolioRepo portfolioRepo, IInstrumentRepo repo, IQuantsAgent quantsAgent, IMetrics metrics, INotificationService notificationService, IHoldingsPublisher holdingsPublisher)
{
 ...
}
 
Hamster said:
Yeah, DI is a beauty until this inevitably happens...

Code: 
class FIXAdapter(ILog log, IUserRepo userRepo, IPortfolioRepo portfolioRepo, IInstrumentRepo repo, IQuantsAgent quantsAgent, IMetrics metrics, INotificationService notificationService, IHoldingsPublisher holdingsPublisher)
{
 ...
}
Click to expand...

Aint that the truth!

However..

The 'guideline' max services that you should need to inject is 5. Why 5? Don't know. That's what a DI book said. But then, there are always exceptions [:whistle:].

When you start injecting lots of services you need to also start questioning whether the class you are injecting into still satisfies SRP. Maybe it needs a refactor.

What I have done in this case is to look into how all those services can be grouped according to other classes that need those same/or subset of services. Then I simply create a new class as a composition of subsets of those services and then inject those subsets into this class and other classes.

Code: 
public class Repos
{
  public Repos(IUserRepo userRepo,IPortfolioRepo portfolioRepo,IInstrumentRepo instrumentRepo)
  {
     this.userRepo = userRepo;
     this.portfolioRepo= portfolioRepo;
     this.instrumentRepo = instrumentRepo;
  }

  public IUserRepo UserRepo=>userRepo;
  public IPortfolioRepo PortfolioRepo=>portfolioRepo;
  public IInstrumentRepo InstrumentRepo=>instrumentRepo;

  private readonly IUserRepo userRepo;
  private readonly IPortfolioRepo portfolioRepo;
  private readonly IInstrumentRepo instrumentRepo;
}

and so forth and then inject:


Code: 
class FIXAdapter(InfrastructureServices infrastructureServices, Repos repos, Agents agents)
 
Hamster said:
Yeah, DI is a beauty until this inevitably happens...

Code: 
class FIXAdapter(ILog log, IUserRepo userRepo, IPortfolioRepo portfolioRepo, IInstrumentRepo repo, IQuantsAgent quantsAgent, IMetrics metrics, INotificationService notificationService, IHoldingsPublisher holdingsPublisher)
{
 ...
}
Click to expand...
When you start to imagine DI as a "Maslow" beauty, everything starts to look like a nail.
FYI Currying and Partial application are easy solutions to this mess; yet the underlying problem is bigger than just code.
 
Pho3nix said:
Could you guys elaborate on this... Or should a new thread be created?
Click to expand...

I hijacked his comment: it's the maturity, skill, experience and moral of the team (read: human factor) that has the biggest influence on the state of the codebase.
 
Hamster said:
I hijacked his comment: it's the maturity, skill, experience and moral of the team (read: human factor) that has the biggest influence on the state of the codebase.
Click to expand...

So having worked for some dev houses and corporate. The bolded bits are the ones that ring true to me.
Sad thing is the stuff Space posted is the first time I've seen such in error handing :(

Need to work in better teams it seems and upskill quite a bit :o :(
 
You must log in or register to reply here.
Back
Top
Sign up to the MyBroadband newsletter
X