1

I Have got the following constructor :

private PicklistHelper(string docId, string prodId, string user, string formProdId, string filepath,string parts)
    {
        this.documentId = docId;
        this.prodId = prodId;
        this.user = user;
        this.filepath = filepath;
        this.parts = parts;
        this.formProdId = formProdId;
        Log.Message("test parts = " + this.parts);
    }

The logmessage does show a value for the parts variable. After that I call a function.

private ArrayList GetPartDatasets()
    {
        Log.Message("test line 1");
        ArrayList picklist = new ArrayList();
        Log.Message("test line 2 " + parts);
        string[] partIds = parts.Split(new[] { ';' });
        Log.Message("test line 3");

the 2nd test line appears in the logs but the parts variable is empty. after that a null ref error appears.

The PicklistHelper object is created in a function which is called multiple times in a row. Any idea what causes the problem ?

Shingala94
  • 404
  • 3
  • 16
  • 1
    Is the `GetPartDatasets()` method in the `PicklistHelper` class? – Andrew_CS Jul 16 '14 at 13:18
  • possible duplicate of [What is a NullReferenceException and how do I fix it?](http://stackoverflow.com/questions/4660142/what-is-a-nullreferenceexception-and-how-do-i-fix-it) – user3613916 Jul 16 '14 at 13:19
  • 1
    Are you using more than one thread (either directly or indirectly via a timer elapse event)? – Geoff Jul 16 '14 at 13:20
  • How are you instantiating `PicklistHelper` objects with a private constructor? – Andrew_CS Jul 16 '14 at 13:21
  • 2
    `parts` is null in `GetPartDatasets` with it shows empty message in your log and throws `null` error on next line – Arjuna Jul 16 '14 at 13:22
  • Have you made sure that you're not passing in `null` for `parts` when you instantiate the `PicklistHelper`? You should show the line of code that you use to create the `PicklistHelper`. – Andrew_CS Jul 16 '14 at 13:28

3 Answers3

0

The problem is that parts is null.

Basically there are two possible causes I can spot from the code you have provided with comments:

  1. If what you say is true and that exact PickListHelper constructor is called at least once in any conceivable case in your application's flow then it is still possible that whatever is calling PicklistHelper is passing null for the parts parameter. This can be checked inside the constructor by something like if (parts == null) throw new ArgumentNullException("parts");, since you require the parts parameter. Otherwise you should find out who/what is calling PicklistHelper.
  2. The method GetPartDatasets is called before the parts field is initialized or after it has been reset to null.
Bas
  • 25,270
  • 7
  • 45
  • 82
0

null strings are a P.I.T.A. In our software we don't handle strings consistentently such that we call methods on null strings.

Try this

private PicklistHelper(string docId, string prodId, string user, string formProdId, string    filepath,string parts)
    {
        this.documentId = docId??    string.Empty;
        this.prodId     = prodId??   string.Empty;
        this.user       = user??     string.Empty;
        this.filepath   = filepath?? string.Empty;
        this.parts      = parts??    string.Empty;
        this.formProdId = formProdId?? string.Empty;

        Log.Message("test parts = " + this.parts);
    }
radarbob
  • 4,610
  • 2
  • 20
  • 30
  • 1
    Does this not mask errors when a null argument was passed by mistake? Perhaps this would be better designed with different constructor overloads or even optional parameters. – Bas Jul 16 '14 at 13:24
  • 1
    The point is to prevent null reference exceptions. More parameter checking may be in order. – radarbob Jul 16 '14 at 13:27
  • 1
    But a NullReferenceException is much better than no error at all when there should be one. – Bas Jul 16 '14 at 13:28
  • That's why you should check the parameters to a method for invalid values before performing the operations inside the method. Find out that the value you're going to work with is null before trying to operate on the variable. – krillgar Jul 16 '14 at 13:33
  • A rogue null reference exception is not better. Now, if `PicklistHelper` caught and threw that, then OK. Even better however would be robust code that handled "empty" properties without blowing up at the least provocation. – radarbob Jul 16 '14 at 13:33
  • P.S. @BasBrekelmans, I like your idea of optional parameters. But in that sense is `string.Empty` any different that any other default value? But in any approach my preference is to at least start with no null strings. – radarbob Jul 16 '14 at 13:41
  • My preference would be that any type in C# has to be made Nullable explicitly (like it is currently implemented with value types). Then we would have a lot less posts on StackOverflow.com and probably a lot less bugs in software. Replacing null with `string.Empty` might be a solution so long as everyone knows that's what you're doing. – Bas Jul 16 '14 at 13:43
  • I'd say "so long as the code knows what it's doing." Here is my favorite line of code: `string.IsNullOrEmpty(someString.Trim())`. We coders somehow want to treat it like it's a value type; and w/in the same class I've seen some strings null and some empty and no where dealing with `null` and `string.Empty` simultaneously. – radarbob Jul 16 '14 at 13:58
0

string parts;

 if (parts != null)
        {
            string[] partIds = parts.Split(new[] { ';' });
            Log.Message("test line 3" + partIds);
        }

It sounds good. :)

regards, Sekhar

sekhar
  • 23
  • 6