1

Trying to tidy up scope and avoid possible multiple calls to RegisterWindowMessage.
Currently have a class used once with the following member

[DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Auto)]
static extern int RegisterWindowMessage(string lpString);

private int m_message = RegisterWindowMessage("MY_MSG");

As we only have one instance this seems ok, but think it would be more tidy to use. With my basic C# understanding this should call RegisterWindowMessage and assign the result to int and not allow it to change.

private const int message = RegisterWindowMessage("MY_MSG");

however attempting to do so leads to a

error CS0133: The expression being assigned to 'someclass.messageEvent' must be constant

so now I'm confused, does this mean the function was being assigned and called each time m_message was used previously, is there something else missing?

Greg Domjan
  • 13,943
  • 6
  • 43
  • 59
  • See [What is the difference between const and readonly?](http://stackoverflow.com/questions/55984/what-is-the-difference-between-const-and-readonly) – heavyd May 26 '10 at 20:25

3 Answers3

7

A const field has to be a compile-time constant. If you just want something which won't change at execution time after initial assignment1, make it readonly:

private static readonly int Message = RegisterWindowMessage("MY_MSG");

Note that I've made this static, which const is implicitly. This means RegisterWindowMessage will only be called once for this AppDomain, which is what I think you want.

EDIT: Hans is right, you should check the return value. You could either do that when you first use it, or when the type is initialized - usually it's a bad idea for type initializers to throw exceptions, but you should see what the impact is.

1 Strictly speaking, a static readonly field can be assigned in the declaration or in the static constructor; an instance readonly field can be assigned in the declaration or in any instance constructor. It can be assigned multiple times, which is usually not useful, but can be just occasionally.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
3

There is another consideration here. RegisterWindowMessage() can fail, you really need to check for that. Using the 0 it returns when something is wrong is going to be horribly difficult to diagnose otherwise.

That knocks initializing it directly in the readonly declaration out. You could use a static constructor instead. Problem with that is that the exception message will be buried in an InnerException. Kinda okay perhaps since failure will be rare.

The best solution is a static property getter that lazily calls the API:

private int m_message

public static int message {
  get {
    if (m_message == 0) {
      m_message = RegisterWindowMessage("blah");
      if (m_message == 0) throw new Win32Exception();
    }
    return m_message;
  }
}

Use well-known locking patterns if this might be called from different threads.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
1

To add to the answer that Hans gave, you can do better than simply throwing an empty Win32Exception. This applies to any API call that uses GetLastError:

[DllImport("user32.dll", SetLastError = true)]
extern static int RegisterWindowMessage(string lpString);

if (m_message == 0)
    throw new Win32Exception(Marshal.GetLastWin32Error());

That will produce a more informative exception.

Tergiver
  • 14,171
  • 3
  • 41
  • 68
  • Hans is correct. I had to think about why I believed it to be not so. If you do not specify SetLastError = true in the p/invoke declaration, the marshaller will not call GetLastError and Marshal.GetLastWin32Error will return 0 which causes the exception message to be, "The operation completed successfully". By specifying Marshal.GetLastWin32Error in the Win32Exception constructor, you are essentially reminding yourself that you need to add SetLastError=true to the declaration. Maybe I'm the only one who needs such a reminder. – Tergiver May 27 '10 at 13:41