views:

273

answers:

4

I inherited a project that needs to be mutithreaded. There is three major classes that get used in the worker threads.

BASE CLASS - has an class level SqlDataAdapter and DataTable. INHERITED CLASS ONE - Uses the inherited SqlDataAdapter and DataTable. INHERITED CLASS TWO - Uses the inherited SqlDataAdapter and DataTable.

Every thing seams to work, but I only have two users testing at the same time.

Is having the SqlDataAdapter and DataTable be class level variables a bad idea?

Update Sorry it's a SqlDataAdapter not a SqlTableAdapter. The language is C#. The SqlDataAdapter and DataTable are from the System.Data.SqlClient namespace.

Here is some of the base class:

public abstract class BaseSync
{
    #region Variables
    internal SqlDataAdapter stageDataAdapter;
    internal DataTable stageDataTable;
    #endregion //Variables
}

Part Two

There is also a singleton utility class that all the derived classes use. I don't know if it will cause problems or not. It looks like this:

public class Utility
{ 
    private static readonly Utility _utility= new Utility();

    private Utility()
    { }

    public static Utility GetUtility()
    {
        return _utility;
    }

    public int GetAutoNumber(string tablename, string fieldname, string siteId)
    {
        string _tablename = tablename;
        string _fieldname = fieldname;
        ...
    }

    internal MissingInfo NormalizeRow(DataRow dataRow)
    {

        MissingInfo retVal = MissingInfo.None;

        //Num
        if (dataRow["Num"] == DBNull.Value)
        {
           retVal =MissingInfo.Num;
           dataRow["Num"] = 1;
        }
        ...
    }
}
+3  A: 

This depends on the access level of the objects. As long as they are not Static (Shared in VB.NET). You should be fine having them in the object, as long as each thread, has its own instance of the object.

Where you come into interesting situations are with the static members that are shared across all instances.

So the long and short of it is that we would need to see the code.

Mitchel Sellers
+2  A: 

Having variables modified by different threads without syncronization is always a really bad idea.

You don't mention if this is the case though. If you thread, you need to plan and check what you're doing.

Marcus Lindblom
+1  A: 

you should always consider doing synchronization when sharing non-constant objects in multi-thread, otherwise you'll end screwed up someday...

so, it's OK if you want to make it a class variable, but remember to make some lock mechanism for it.

Francis
+2  A: 

The rule about variables is that the more places they could potentially change from, the more chances there are of race conditions, especially if the application evolves.

There isn't much informtion in your question so it is difficult to provide a specific answer. Class-level variables (if public) can often be treated like global variables and are thus accessible from everywhere, raising the risk of corruption.

A possible approach might be to hide these fields and at provide access via class-level functions. You can then do more things because you've created specific points of access to these variables. You would need to be careful to ensure that you are never giving applications a direct and mutable references to that object, which may require some rewriting, but it would make your program safer.

Uri