Klasa do połączenia z bazą - jak elegancko

Witam, piszę klasę, której zadaniem będzie łączyć się z bazą danych mysql.

Oto ona:

 
public class Database
    {
        public static MySqlConnection connection;
        public static string database;
        public static string server;
        public static string user;
        public static string password;

        public static bool Connect()
        {
            try
            {
                connection.ConnectionString = "database=" + database + "; server=" + server + "; user id=" + user + "; Password=" + password;
                return true;
            }
            catch
            {
                return false;
            }
        }
    }

Jednak powiem szczerze, że nie podoba mi się, że zmienne są statyczne, raczej nie tędy droga, jakieś pomysły?

Pozdrawiam

Od biedy można zastosować wzorzec singleton. Często się go stosuje przy bazach danych, aczkolwiek mi samemu się on nie podoba.
Poza tym jeśli wiesz, że zmienne nie powinny być statyczne, to... zmień je. :)

No tak, ale jeżeli mam statyczną metodę, to zmienne też muszą być statyczne w moim przypadku statyczne :-) Dodałem klasę statyczną, bo nie będzie potrzeby tworzyć jej instancji.

       public static class Database
    {
        public static MySqlConnection connection;
        public static string database="xxxx";
        public static string server = "xxxx";
        public static string user = "xxxx";
        public static string password = "xxx
        public static bool Connect()
        {
            try
            {
                connection = new MySqlConnection();
                connection.ConnectionString = "database=" + database + "; server=" + server + "; user id=" + user + "; Password=" + password;
                connection.Open();
                return true;
            }
            catch
            {
                return false;
            }
        }
    }

Czytałem o singletonie, ale on mi się też nie podoba, wolę coś tego typu.

Wywołuję tą metodę w programie głównym tak:

 
bool isConnect = Database.Connect();

Nie pokazuje się żaden błąd, ale program raczej nie łączy się prawidłowo :-) Jak myślisz gdzie może być błąd?

Forma:

 
public partial class mainWindow : Form
    {
        bool isConnect = false;

        public mainWindow()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            connect.RunWorkerAsync();
            
        }

        private void connect_DoWork(object sender, DoWorkEventArgs e)
        {
            isConnect = Database.Connect();
            toolStripStatusLabel1.Text = "Status: Łączę...";
        }

        private void connect_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
        {
            if (isConnect)
            {
                toolStripStatusLabel1.Text = "Status: Połączono!";
            }
            else
                toolStripStatusLabel1.Text = "Status: Niepołączono...";
        }
    }

Dobra już wiem dlaczego nie mogę się połączyć :-) Po prostu nie połącze się z tą bazą mysql z zewnątrz i tyle.

Pozdrawiam

Singleton jest lepszy niż metody i zmienne statyczne pomimo tego, że mi się nie podoba. Metody i zmienne statyczne w takim przypadku jak Twój to najgorsze co może być i szlag mnie wręcz trafia jak coś podobnego widzę. Jak napiszesz testy jednostkowe dla Twojej klasy w takiej postaci? Jakoś napiszesz, ale będą to testy złej jakości.

Statyczne funkcje działające na bazie danych zmieniają status działania na tej bazie, przez co testy jednostkowe są niewiarygodne, a działanie programu nieprzewidywalne, bo nigdy nie wiadomo, czy przypadkiem stan jakiejś zmiennej nie został zmienione w niewiadomym miejscu aplikacji.

Z tego też powodu powinno się pisać tym podobne klasy w pełni obiektowo lub przynajmniej zastosować ten pieprzony singleton.
Kiedyś też słyszałem o wstrzykiwaniu zależności w takim przypadku, ale nie wiem czy dobrze kojarzę i nie stosowałem w praktyce.

u Ciebie to cudo:
public static MySqlConnection connection;

może być zmodyfikowane w każdej chwili przez każdą rzecz w programie. W jaki sposób, chcesz więc zapewnić bezpieczeństwo i niezawodność?
Nie dasz rady po prostu bo to nie możliwe w tak napisanym kodzie.

Ta funkcja to też perełka:

public static bool Connect()
        {
            try
            {
                connection = new MySqlConnection();
                connection.ConnectionString = "database=" + database + "; server=" + server + "; user id=" + user + "; Password=" + password;
                connection.Open();
                return true;
            }
            catch
            {
                return false;
            }
        }

Na wejściu tworzysz nowy obiekt, a Twoja klasa właśnie jest do kitu, bo być może już taki obiekt masz. W ten sposób wywołanie w pętli funkcji Vonnect będzie w kółko tworzyć obiekty i łączyć się do bazy.
Dlaczego tak zrobiłeś? Z prostego powodu - bo nie mogłeś sprawdzić stanu połączenie, gdy był null. Wymagało to zbyt wielu rozgałęzień w kodzie, więc postawiłeś na wersję prostą, ale... bezsensowną własnie z powodu statyczności.

Funkcje statyczne powinny być stosowane tylko wtedy, gdy nie korzystają z atrybutów danej klasy. Np funkcja:
int Math.Add(int x, int y)

Rozumiem, postaram się to poprawić. Czy są jakieś ograniczenia założone przez programistów ( chodzi o to co winno a czego nie winno ) co do kodowania funkcji w Singletonie? Można w niej zakodować funkcję boolowską czy nie? Chodzi o dobrą, złą praktykę.

Singleton to cała klasa...
Możesz sobie w ramach takiej klasy, zdefiniować całą armię funkcji "boolowskich" (cokolwiek to oznacza).

W bardzo dużym skrócie: Singleton to zwykła klasa, tylko trzyma swoją instancję pod zmienną statyczną i ma prywatny konstruktor(czyli nie da się wywołać new).
(jak zerkniesz na źródło singletona to zrozumiesz: http://www.dofactory.com/Patterns/PatternSingleton.aspx#_self1)
Zamiast tworzenia nowej instancji, pobierasz (jedyną) istniejącą już instancję ze zmiennej statycznej.

Tu jest wszystko opisane:
http://msdn.microsoft.com/en-us/library/ff650316.aspx

kod z podanego wyżej adresu:

public sealed class Singleton
{
   private static readonly Singleton instance = new Singleton();
   
   private Singleton(){}

   public static Singleton Instance
   {
      get 
      {
         return instance; 
      }
   }
}
 

 
class MySingleTon
    {
        private static MySingleTon singleTonObject;

        private MySingleTon()
        {

        }

        public System.Data.OleDb.OleDbConnection connection;

        public bool Connection()
        {
                connection = new OleDbConnection();
                connection.ConnectionString = "PROVIDER=Microsoft.ACE.OLEDB.12.0; Data Source=" + Application.StartupPath + "\\moja_baza.accdb";
                connection.Open();
                return true;
        }

        public static MySingleTon CreateInstance()
        {
            object myObject = new object();

            lock (myObject)
            {
                if (singleTonObject == null)
                    singleTonObject = new MySingleTon();
            }
            return singleTonObject;
        }
    }

Rozumiem już ten wzorzec, ale mam jeszcze inny kłopot. Klasa, o nazwie Warzywa potrzebuje obiektu "connection", który tworzy metoda Connection. W jaki sposób mogę przekazać ten obiekt aby do klasy Warzywa ( oczywiści już stworzony, aby nie było błędu, że zmienna nie została zainicjowana?? ) W konstruktorze mogę przekazać klasę MySingleton ( utworzoną jej instancję ) ale wole zapytać, bo mam trochę dużo klas i nie wiem czy warto się bawić bo jak nie zadziała :-)

	class DbConnection
	{
		private static DbConnection _instance;
		private static object _lock = new object();

		public OleDbConnection Connection { get; private set;}


		private DbConnection()
		{
			Connection = new OleDbConnection() { ConnectionString = "PROVIDER=Microsoft.ACE.OLEDB.12.0; Data Source=" + Application.StartupPath + "\\moja_baza.accdb" };
			Connection.Open();
		}


		public static DbConnection Instance
		{
			get
			{
				if (_instance == null)
				{
					lock (_lock)
					{
						if (_instance == null)
							_instance = new DbConnection();
					}
				}
				return _instance;
			}
		}

	}

tak to według mnie powinno wyglądać. masz źle zrobiony lock (wywołujesz go na lokalnej zmiennej, która przy każdym wywołaniu metody jest tworzona na nowo) i popełniasz kardynalny błąd: wystawiasz publiczną metodę Connect, która naprawdę inicjalizuje Singleton, a to przecież o to chodziło, żeby ja schować, po to właśnie został użyty ten wzorzec.

obiekt connection podasz metodzie poprzez DbConnection.Instance.Connection.