C# Hashtable in singleton retourneerd Null op aanroep

[Rarz]

Hola,

ik heb een singleton class die als verzamelplaats dient voor allerlei variabelen die zo nu en dan aangepast worden. Voornamelijk strings. Er zit tevens een Hashtable in die op een gegeven moment gevult dient te worden, en daar gaat het fout;

C#:

using System;
using System.Collections.Generic;
using System.Text;
using System.Collections;

namespace ZipGen
{
    public class WorkItem
    {
        private static WorkItem _instance;

        private string _searchkey;
        private Hashtable _directoryfilename;

        private WorkItem()
        {
        }

        /// Static constructor
        public static WorkItem Instance
        {
            get
            {
                if (_instance == null)
                {
                    _instance = new WorkItem();
                }
                return _instance;
            }
        }

        // Properties
        public string SearchKey
        {
            get { return _searchkey; }
            set { _searchkey = value; }
        }
        public Hashtable DirectoryFilenameHashtable
        {
            get { return _directoryfilename; }
            set { _directoryfilename = value; }
        }
    }
}

Elders in het programma worden wat waardes weggeschreven in singleton - stings, ints, gaat allemaal prima. Maar als ik probeer om Key/Values aan de Hashtable te .add-en, klapt het programma met de mededeling dat er een Nullreferentie heeft plaatsgevonden. En inderdaad, de hashtable blijkt null te zijn - maar ik zie niet waarom. Hij is exposed en heeft een Get.

De SQL werkt, en de reader retourneerd geen null of anderzijds onbruikbare data (gewoon twee strings). De reden om het in een Hashtable te stoppen is snelheid.

C#:

        static void LoadDirectoryTranslationTable () {

            WorkItem wi = WorkItem.Instance; // Instance of WorkItem

            using (SqlConnection connection = new SqlConnection(connectionstring))
            {
                SqlCommand command = new SqlCommand("SELECT charDirectory, charFilename FROM characters", connection);
                connection.Open();

                SqlDataReader reader = command.ExecuteReader();

                while (reader.Read())
                {
                    wi.SearchKey = "somestring"; // works!
                    //Console.WriteLine(reader.GetString(0) + " " + reader.GetString(1)); // Works! Resultset aanwezig
                    wi.DirectoryFilenameHashtable.Add(reader[0], reader[1]); // Klapt. Reader bevat data, maar 'DirectoryFilenameHashtable' is Null
                }

                // Call Close when done reading.
                reader.Close();
            }
        }

Suggesties?

[Verwijderd]

vooralsnog zie ik geen code waarmee je je hashtable aanmaakt.

C#:

private WorkItem() 
{ 
    _directoryfilename = new Hashtable();
} 

zoiets scheelt misschien?

[Rarz]

Oh, doh! String begint met default instance, Hashtable moet specifiek nog geinstantieerd worden.

Leermomentje - en dank.

[bigbeng]

Zie het maar als good practice om je variabelen altijd te initialiseren. Over het algemeen geeft de compiler hier ook warnings over.

Een ander onderwerp om meer over te lezen is het verschil tussen value en reference type en het bijzondere gedrag van strings (is een reference type, maar gedraagt zich in een aantal opzichten als een value type). En boxing/unboxing is ook een interessant onderwerp, als je toch bezig bent .

Ik weet niet of het wat is voor jou, maar ik vond hier een tutorial voor c# die ook deze onderwerpen behandelt:
http://www.softsteel.co.uk/tutorials/cSharp/cIndex.html

[Confusion]

bigbeng schreef op maandag 08 september 2008 @ 09:02:
Zie het maar als good practice om je variabelen altijd te initialiseren.

Dat zie ik zeker niet als good practice. Als mijn applicatie een configuratiefile niet kan vinden, dan wil ik dat mijn "public static Configuration config" gewoon null blijft. Dan valt er in ieder geval snel genoeg een NullPointerException als ik vergeet daarop te controleren. Dat is een stuk inzichtelijker dan ugh lang moeten zoeken waarom bepaalde waarden in de config file niet doorkomen, om er uiteindelijk achter te komen dat de hele file uberhaupt niet geladen worden en je zelf een default leeg Configuration object had geconstrueerd. (En bij dat laatste vraagt iedereen die de code leest zich ook af: WTF?)

[Woy]

Rarz schreef op maandag 08 september 2008 @ 07:34:
Oh, doh! String begint met default instance, Hashtable moet specifiek nog geinstantieerd worden.

Leermomentje - en dank.

Een string is default ook null hoor. Alleen Value Types worden als instance variabelen op hun default geinitialiseerd.

[whoami]

Tja, als je nu eens zelf eerst de code gedebugged had, dan had je toch zelf al kunnen zien dat je die hashtable nergens initialiseerde ?
Eigenlijk hoef je dit zelfs niet te debuggen om al een groot vermoeden te hebben dat die hashtable nergens ge-init werd ...

Confusion schreef op maandag 08 september 2008 @ 09:12:
[...]

Dat zie ik zeker niet als good practice. Als mijn applicatie een configuratiefile niet kan vinden, dan wil ik dat mijn "public static Configuration config" gewoon null blijft. Dan valt er in ieder geval snel genoeg een NullPointerException als ik vergeet daarop te controleren. Dat is een stuk inzichtelijker dan ugh lang moeten zoeken waarom bepaalde waarden in de config file niet doorkomen, om er uiteindelijk achter te komen dat de hele file uberhaupt niet geladen worden en je zelf een default leeg Configuration object had geconstrueerd. (En bij dat laatste vraagt iedereen die de code leest zich ook af: WTF?)

Mja .... Je zou in sommige gevallen ook het 'NullObject' pattern kunnen overwegen vind ik. 't Hangt allemaal af van de situatie; als je configuratie file / settings dingen nodig heeft die echt noodzakelijk zijn om met de applicatie te kunnen werken (connectiestring settings oid), dan zou je idd een exceptie kunnen geven, maar als je config file geen 'levensnoodzakelijke dingen' bevat, dan kan je evengoed gebruik maken van het NullObject pattern, en een config object returnen met wat default settings.

[bigbeng]

Confusion schreef op maandag 08 september 2008 @ 09:12:
[...]

Dat zie ik zeker niet als good practice. Als mijn applicatie een configuratiefile niet kan vinden, dan wil ik dat mijn "public static Configuration config" gewoon null blijft. Dan valt er in ieder geval snel genoeg een NullPointerException als ik vergeet daarop te controleren. Dat is een stuk inzichtelijker dan ugh lang moeten zoeken waarom bepaalde waarden in de config file niet doorkomen, om er uiteindelijk achter te komen dat de hele file uberhaupt niet geladen worden en je zelf een default leeg Configuration object had geconstrueerd. (En bij dat laatste vraagt iedereen die de code leest zich ook af: WTF?)

Eens, maar op null initialiseren zie ik ook als initialiseren. Dat was misschien niet duidelijk, zie dit als een aanvulling op wat ik eerder schreef.

Goed coderen bevat mijns inziens trouwens ook het zorgen dat een kritieke fout leidt tot een exception. Een ongecontroleerde null-pointer exception vind ik dan ook weer een bad practice. Het niet kunnen lezen van een configuratiefile lijkt me een uitstekende reden om de program flow te onderbreken.

[Jaap-Jan]

bigbeng schreef op maandag 08 september 2008 @ 09:45:
[...]

Eens, maar op null initialiseren zie ik ook als initialiseren. Dat was misschien niet duidelijk, zie dit als een aanvulling op wat ik eerder schreef.

(...)

Dan heb je een kromme definitie van 'initialiseren' imho. Het betekent namelijk 'klaarmaken voor gebruik'. En als je een object op null 'initialiseert', dan moet je nog steeds een nieuw object aanmaken voordat je hem kunt gebruiken. Het enige wat je hebt gedaan is je object definiëren. Null is geen waarde, het is de afwezigheid ervan

[ Voor 23% gewijzigd door Jaap-Jan op 08-09-2008 09:57 ]

[Confusion]

bigbeng schreef op maandag 08 september 2008 @ 09:45:
Eens, maar op null initialiseren zie ik ook als initialiseren. Dat was misschien niet duidelijk, zie dit als een aanvulling op wat ik eerder schreef.

OK, in Java worden alle instance variabelen standaard geinitialiseerd op null (voor de niet-primitives), dus het leek mij dat _directoryfilename sowieso op null geinitialiseerd was.

Een ongecontroleerde null-pointer exception vind ik dan ook weer een bad practice. Het niet kunnen lezen van een configuratiefile lijkt me een uitstekende reden om de program flow te onderbreken.

Natuurlijk, maar als je zo'n check zou vergeten (en dat gebeurt iedereen weleens), dan moet het programma goed hard onderuit gaan en niet met foute data (want '0' of "" kan gewoon echt fout zijn) blijven doorlopen.

whoami schreef op maandag 08 september 2008 @ 09:20:
Mja .... Je zou in sommige gevallen ook het 'NullObject' pattern kunnen overwegen vind ik. 't Hangt allemaal af van de situatie; als je configuratie file / settings dingen nodig heeft die echt noodzakelijk zijn om met de applicatie te kunnen werken (connectiestring settings oid), dan zou je idd een exceptie kunnen geven, maar als je config file geen 'levensnoodzakelijke dingen' bevat, dan kan je evengoed gebruik maken van het NullObject pattern, en een config object returnen met wat default settings.

Als er 'sane defaults' zijn, dan ben ik het daar helemaal mee eens. Het risico is alleen dat je defaults gaat verzinnen die 'sane' lijken, maar dat eigenlijk helemaal niet zijn. Testconfiguratie als default gebruiken is daar een voorbeeld van. Ik heb al eens meegemaakt dat een productie applicatie ongemerkt tegen de testdatabase aan stond te ouwehoeren, omdat iemand dacht dat 'test' settings wel een 'sane default' waren.

[bigbeng]

Jaap-Jan schreef op maandag 08 september 2008 @ 09:57:
[...]


Dan heb je een kromme definitie van 'initialiseren' imho. Het betekent namelijk 'klaarmaken voor gebruik'. En als je een object op null 'initialiseert', dan moet je nog steeds een nieuw object aanmaken voordat je hem kunt gebruiken. Het enige wat je hebt gedaan is je object definiëren. Null is geen waarde, het is de afwezigheid ervan

Eentje die gedeeld wordt door Microsoft Compileer het volgende blokje code maar eens:

C#:

public void Dummy()
{
   SqlConnection conn;
   try
   {
      conn = new SqlConnection("maakt voor dit voorbeeld niet uit");
      conn.Open();
   }
   finally
      if ( conn != null )
      {
         conn.Close();
      }
   }
}

Bij mij geeft dat de volgende error:
"Use of unassigned local variable 'conn'"
Dit in VS2005 en VS2008.

[Janoz]

'unassigned' is niet hetzelfde als 'uninitialized'. 'Assignen' is engels voor toewijzen. De foutmelding geeft aan dat er mogelijk niks toegewezen is aan conn (wat mogelijk is wanneer er een exceptie optreed in de constructor van SqlConnection), niet dat conn niet geïnitialiseerd is.

Java geeft trouwens dezelfde foutmelding/waarschuwing.

[ Voor 9% gewijzigd door Janoz op 08-09-2008 14:10 ]

[Woy]

bigbeng schreef op maandag 08 september 2008 @ 14:02:
[...]

Eentje die gedeeld wordt door Microsoft Compileer het volgende blokje code maar eens:

C#:

public void Dummy()
{
   SqlConnection conn;
   try
   {
      conn = new SqlConnection("maakt voor dit voorbeeld niet uit");
      conn.Open();
   }
   finally
      if ( conn != null )
      {
         conn.Close();
      }
   }
}

Bij mij geeft dat de volgende error:
"Use of unassigned local variable 'conn'"
Dit in VS2005 en VS2008.

Maar de volgende code geeft die error niet

C#:

SqlConnection conn;
public void Dummy()
{   
   try
   {
      conn = new SqlConnection("maakt voor dit voorbeeld niet uit");
      conn.Open();
   }
   finally
      if ( conn != null )
      {
         conn.Close();
      }
   }
}

Ik heb het altijd wat apart gevonden dat Class variabelen anders worden behandeld als lokale variabelen

[ Voor 5% gewijzigd door Woy op 08-09-2008 14:11 ]

[Janoz]

@rwb: Nogal logisch gezien de scope van de variabele conn. Het zou juist vreemd zijn wanneer .NET zou verlangen dat je aan conn iets toegewezen moet hebben.

[4of9]

bigbeng schreef op maandag 08 september 2008 @ 09:02:
Zie het maar als good practice om je variabelen altijd te initialiseren. Over het algemeen geeft de compiler hier ook warnings over.

Als je FxCop gebruikt zul je zien dat microsoft juist het tegenovergestelde vind:

http://msdn.microsoft.com/nl-nl/library/ms182274(en-us).aspx

FxCop (en dus microsoft) ziet het als een badpractice om the initializeren als de compiler dit als voor je doet. (dus op null initializeren, of bools op false initializeren e.d)

The common language runtime initializes all fields to their default values before running the constructor. In most cases, initializing a field to its default value in a constructor is redundant, which degrades performance and adds to maintenance costs. One case where it is not redundant occurs when the constructor calls another constructor of the same class or a base class constructor and that constructor initializes the field to a non-default value. In this case, changing the value of the field back to its default value can be appropriate.

[BramFokke]

Nou ben ik benieuwd wat er in die MS recommendation wordt bedoeld met 'constructor'. Is dat alleen een expliciet gedeclareerde constructor? Of gaat het ook om alle initialisatiewaarden in de variabeledefinitie?

C#:

public class Koekje {
    // Is geen onderdeel van de expliciete constructor,
    // maar wordt wel aangeroepen bij aanmaken object.
    public int SesamZaadjes = 0;

    public Koekje() {
        SesamZaadjes = 0;
    }
}

Ik kan me gevallen voorstellen waarin in beide gevallen het expliciet opnemen van een default waarde de leesbaarheid van de code ten goede komt.

[Woy]

Janoz schreef op maandag 08 september 2008 @ 14:12:
@rwb: Nogal logisch gezien de scope van de variabele conn. Het zou juist vreemd zijn wanneer .NET zou verlangen dat je aan conn iets toegewezen moet hebben.

Ja het is wel logisch ( heb ik er ook onder staan ), want in de situatie die ik schets is het natuurlijk voor de compiler niet goed te controleren of de variabele al assigned is.

Wat ik meer apart vind, is dat locale variabelen ook niet gewoon op null geinitialiseerd worden zodat het gedrag gelijk is aan class fields.

[gorgi_19]

rwb schreef op maandag 08 september 2008 @ 14:10:
[...]

Maar de volgende code geeft die error niet

C#:

SqlConnection conn;
public void Dummy()
{   
   try
   {
      conn = new SqlConnection("maakt voor dit voorbeeld niet uit");
      conn.Open();
   }
   finally
      if ( conn != null )
      {
         conn.Close();
      }
   }
}

Ik heb het altijd wat apart gevonden dat Class variabelen anders worden behandeld als lokale variabelen

C#:

public void Dummy() 
{ 
   SqlConnection conn = null; 
   try 
   { 
      conn = new SqlConnection("maakt voor dit voorbeeld niet uit"); 
      conn.Open(); 
   } 
   finally 
      if ( conn != null ) 
      { 
         conn.Close(); 
      } 
   } 
}

Zou die waarschuwing ook niet moeten geven....

[whoami]

4of9 schreef op maandag 08 september 2008 @ 14:36:
[...]


Als je FxCop gebruikt zul je zien dat microsoft juist het tegenovergestelde vind:

http://msdn.microsoft.com/nl-nl/library/ms182274(en-us).aspx

Dat is nu eens één van de regels van fxcop waar ik het niet mee eens ben, en die dus standaard uitgeschakelt staat bij mij.

Ik vind het in bepaalde gevallen nl. duidelijker / explicieter om een variable echt 'hard' te gaan initializeren, ook al init ik die variable op de 'default' waarde.
Stom voorbeeld:

C#:

bool found = false;

foreach( Melp m in SomeCollection )
{
      if( m.Id = bliep ) found = true;
}

[Janoz]

Van een lokale variabele is de scope exact bekend waardoor de controle ook redelijk makkelijk is. Door de toekenning af te dwingen wordt je automatisch gedwongen om even na te denken voordat je aan je variabele null toekent.

Een aparter voorbeeld vind ik zelf:

code:

some method(params){
  ObjectWithSomeMethod o;
  if (someBooleanExpression) {
    o = new Ditje();
  } else {
    o = new Datje();
  }
  o.someMethod()
}

Bij Java krijg je hier ook de unassigned foutmelding, ondanks dat er eigenlijk altijd wel wat aan o toegekend is.

@BramFokke

Met die recomendation bedoelen ze het volgende:

code:

public class Koekje {
    // Is geen onderdeel van de expliciete constructor,
    // maar wordt wel aangeroepen bij aanmaken object.
    public int SesamZaadjes = 0;

    public Koekje(Ditje di) {
      this(datje)
      //something met di;
      SesamZaadjes = 0;       
    }

    public Koekje(Datje da){
      SesamZaadjes = 213;
    }
}

Persoonlijk zou ik me geen enkele situatie voor kunnen stellen waarbij het handig is om een constante op twee plaatsen te definiëren.

[4of9]

Ik moet zeggen dat ik het ook altijd anders heb geleerd hoor. We zijn ook nog bezig om FXCop te leren kennen maar ik stond er wel van te kijken toen deze regel een melding gaf...

[Janoz]

whoami schreef op maandag 08 september 2008 @ 16:18:
Ik vind het in bepaalde gevallen nl. duidelijker / explicieter om een variable echt 'hard' te gaan initializeren, ook al init ik die variable op de 'default' waarde.
Stom voorbeeld:

C#:

bool found = false;

foreach( Melp m in SomeCollection )
{
      if( m.Id = bliep ) found = true;
}

Het door jou gegeven voorbeeld is toch iets waarbij het juist wel afgedwongen wordt? found niet op false zetten zou juist een error/warning geven.

[whoami]

Nee, wat ik wil aangeven, is dat ik het voorbeeld dat ik gaf idd duidelijker vind.
Als ik 'found' niet initialiseer, dan gaat de compiler daar (in C#) geen error of warning op geven. Found krijgt nl. impliciet de waarde 'false'. (Value types worden op hun default waarde ge-init).

Echter, als je je code door FxCop haalt, dan zal FxCop daarover wel gaan klagen, omdat je die variable onnodig initialiseert . (Zie de link van 4of9).
En dat vind ik 'dwaas', en daarom disable ik die specifieke fxcop rule.

[Janoz]

Owh, dan is het inderdaad dwaas. Ik had even de java manier geprojecteerd. Daar krijg je een foutmelding wanneer je hem niet initialiseert.

[BramFokke]

Persoonlijk zou ik me geen enkele situatie voor kunnen stellen waarbij het handig is om een constante op twee plaatsen te definiëren.

Daar ben ik het helemaal mee eens. Maar er zijn wel situaties waar het leesbaarder is om expliciet een waarde op te geven die een default waarde is. Bijvoorbeeld als het gaat om een initiële waarde van een algoritme of om een waarde die contra-intuïtief is.

[Janoz]

Noem eens een paar situaties? Het op twee plekken neerzetten van een default waarde zorgt er alleen maar voor dat er gegarandeerd ergens in de toekomst een keer iemand is die het aanpast en dan 1 van beiden vergeet.

[BramFokke]

Janoz schreef op maandag 08 september 2008 @ 16:59:
Noem eens een paar situaties? Het op twee plekken neerzetten van een default waarde zorgt er alleen maar voor dat er gegarandeerd ergens in de toekomst een keer iemand is die het aanpast en dan 1 van beiden vergeet.

Ik had het over het expliciet initialiseren op een default-waarde. Het stukje code dat ik had geschreven was bedoeld om onderscheid te maken tussen het initialiseren van variabelen in de declaratie en in de expliciete constructor. Het op twee plaatsen definiëren van dezelfde waarde is inderdaad vragen om problemen, daar zijn we het denk ik wel over eens

[Janoz]

Ok, in dat geval ben ik nu alweer voor de 2e keer in dit topic langs iemand heen aan het praten

[BramFokke]

Janoz schreef op maandag 08 september 2008 @ 19:18:
Ok, in dat geval ben ik nu alweer voor de 2e keer in dit topic langs iemand heen aan het praten

Haha no problem, het is altijd leuk (en spoms zelfs zinvol ) om te bakkeleien over dit soort vraagstukken omdat iedereen er weer een andere kijk op nahoudt.

[Verwijderd]

Ik denk dat het instantieren van een object alleen daar waar nodig is ook een aantal voordelen heeft. Je kan gewoon testen op null en het komt de performantie ten goede.

C#:

MyClass GetResult()
{
 MyClass retResult = null;

  if (SomeChecks())
   {
    retResult = new MyClass();
    retResult.DoSomething();
   }

 return retResult;
}

[EfBe]

Je singleton is ook niet threadsafe.

Doe:
private static WorkItem _instance - new WorkItem();

ipv het te instantieren in een static ctor. Zie:
http://www.yoda.arachsys.com/csharp/singleton.html