tags:

views:

211

answers:

4

Hi all,

my form1 class contains a bunch of data that I need to save, so there should only be one instance of it running at a time.

public partial class Form1 : Form
{
    public string form1string = "I really need to save this data";

    public Form1()
    {
        InitializeComponent();


        // Even if I pass my form1 object here I still can't access it from
        // the upcoming static methods.
        InterceptKeys hook = new InterceptKeys();
    }

InterceptKeys, which is not my code, contains a bunch of static methods required for keyboard hooks.

private static IntPtr HookCallback(int nCode, IntPtr wParam, IntPtr lParam)
{
    if (nCode >= 0 && wParam == (IntPtr)WM_KEYDOWN)
    {
       int trueKeyPressed = Marshal.ReadInt32(lParam);

       if (Form1.mirror)
       {
           Form1.newKeyPress(trueKeyPressed);
           return (IntPtr)1;
       }
    }
    return CallNextHookEx(_hookID, nCode, wParam, lParam);
 }

Because the HookCallBack method is static, Form1.newKeyPress() needs to be static as well.

But if newKeyPress in static, I can't access the data that I need! I wouldn't want to declare a new form1 object here, as that would give me different versions of the data. Correct?

I'm not an Object Oriented Expert. How should I format this, to ensure that all of the InterceptKey's form1 method calls are going to THE form1 object that I want?

Thanks, please let me know if there's any more info that you need!

+1  A: 

From what you've written, I think this will work:

In Form1, have a static member of type Form1 which will hold the instance :

private static Form1 instance;

In the Form1 constructor, set this static member to the instance being created:

public Form1()
{
    // Existing code

    Form1.instance = this;
}

Now, make newKeyPress static, and use the static member to find the actual form instance:

public static void newKeyPress(int trueKeyPressed)
{
    Form1 thisIsTheForm1Instance = Form1.instance;

    // Now instance.form1string is the data you want
}
AakashM
-1: static instance != one instance of form open at a given time. Case in point: consider a dialog which opens and closes multiple times throughout the lifetime of an app, where we instantiate a new dialog if its not showing and focus/activate the existing dialog otherwise. There are almost no cases where a static form is preferable to a non-static one.
Juliet
Interesting. I don't care about the concept of having only one instance so much as the execution, that is, always being able to access my data. Will your point here keep me from doing that? If so, do you have a better suggestion given the constraints?
cksubs
+1  A: 

I think something like having in the Form1 class:

private static Form1 instance;


 public static Form1 Instance
 {
      get
      {
          if (instance == null)
          {
              instance = new Form1();
          }
          return instance;
      }
 }

And in your classes which use them:

Form1 form1 = Form1.Instance;

Will do the job. This will first check if there is one, if so it will return the instance else it will create one and next time it will return one.

bastijn
+3  A: 

You have two design issues:

How to call an instance method from a static method

Because the HookCallBack method is static, Form1.newKeyPress() needs to be static as well.

You can pass in an instance of your main form to your HookCallBack method, you'd just need to add an additional parameter to your static method:

private static IntPtr HookCallback(int nCode, IntPtr wParam, IntPtr lParam, Form1 form)
{
   // ...
}

This is actually the preferred strategy. Wherever your methods and classes have dependencies on other objects, you should pass your dependencies into your methods instead of pulling them from global state.

Barring that, you can loop through the Application.OpenForms collection and find the form you're looking for as follows:

var form = Application.OpenForms.OfType<Form1>().First();
form.newKeyPress();

How to have one instance of a form open at one time

Other people have suggested making your form static -- that's one approach, but its a bad approach. Static forms don't get garbage collected when their disposed, you have to implement your own init/reset methods when you show/hide the form, if the static form has references to other objects then your app will slowly leak memory, among other things. I actually recommend something like this:

class FormFactory
{
    public Form1 GetForm1()
    {
        return Application.OpenForms.OfType<Form1>().FirstOrDefault ?? new Form1();
    }
}

So your FormFactory controls the lifetime of your form, now you can get an existing or new instance of Form1 using new FormFactory.GetForm1().

Juliet
Oh I like that. I didn't go that route because I couldn't access any instance variables from `HookCallBack`, but I forgot that you could pass it as a parameter when that is originally called. The only problem is that I didn't write that code, and it looks like `private static LowLevelKeyboardProc _proc = HookCallback;` is how it is called. How do I add the form parameter to a call like that?
cksubs
@cksubs: `LowLevelKeyboardProc` is a delegate (presumably) defined in your project, something like `publc delegate IntPtr LowLevelKeyboardProc(int nCode, IntPtr wParam, IntPtr lParam)`. You need to find the definition of the delegate (most of the time you can right-click and choose "Go to Definition") and add your Form param to the param list. Compile your project, you should get one or more of compilation errors indicating everywhere you need to pass in your new Form param.
Juliet
@cksubs: now that I think about, my suggestion above is just silly. A significant refactor above implies that we're taking the problem entirely wrong. This answer solves your problem equally well with the benefits that there are no modifications to your API: http://stackoverflow.com/questions/2070481/how-do-i-call-an-instance-method-from-another-classes-static-method-i-e-have-o/2070864#2070864
Juliet
+2  A: 

Passing keypresses to other forms

It occurs to me that you're basically just passing keypresses to your form, which implies a kind of basic message notification / observer pattern. Maybe you just need a better design pattern. Try the following:

public class MessageHooks
{
    public static event Action<int> OnHookCallback;

    private static IntPtr HookCallback(int nCode, IntPtr wParam, IntPtr lParam)
    {
        if (nCode >= 0 && wParam == (IntPtr)WM_KEYDOWN)
        {
            int trueKeyPressed = Marshal.ReadInt32(lParam);
            if (OnHookCallBack != null)
            {
                OnHookCallback(trueKeyPressed);
            }
        }
        return CallNextHookEx(_hookID, nCode, wParam, lParam);
    }
}

Guess what? Now your HookCallBack method doesn't even need to know of the existence of forms. Instead, your forms register themselves to the event handler as such:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        MessageHooks.OnHookCallBack += MessageHooks_OnHookCallBack;
        this.FormClosed += (sender, e) => MessageHooks.OnHookCallBack -= MessageHooks_OnHookCallBack; // <--- ALWAYS UNHOOK ON FORM CLOSE
    }

    void MessageHooks_OnHookCallBack(int keyPressed)
    {
        // do something with the keypress
    }
}

Remember, you must unhook your forms event handler on form close, otherwise the form won't get garbage collected, and you'll run into lots of weird issues raising events on non-visible forms.

Juliet
I don't know your requirements, but I should probably comment that my code above isn't thread-safe (i.e. in a race condition, our event handler might pass the null check, but still throw a null-reference exception when we invoke it). See http://stackoverflow.com/questions/786383/c-events-and-thread-safety
Juliet
I like this and the event-driven model is actually how I wanted to do this, so I'm glad to make the change... having trouble though. I noticed you took my `InterceptKeys hook = new InterceptKeys();` out of the form1 constructor, was that deliberate? Without it there I'm not hooking the keyboard at all, with it there I'm not getting into the `if (OnHookCallBack != null)` statement. Am I missing something?
cksubs
Oh, also, the capitalization of the "Backs" up there is another potential source of error. I've standardized them so it compiles, but I hope I didn't mess it up somehow because of that.
cksubs
I also changed all `MessageHooks` to `InterceptKeys` as that's what the class was previously named.
cksubs
Juliet must be sleeping... anyone else notice something I'm missing in this code? So close...
cksubs
*"I noticed you took my InterceptKeys hook = new InterceptKeys(); out of the form1 constructor, was that deliberate?"* No, it was just some ad-hoc, spontaneously written code. Add your method back if you need it. *Oh, also, the capitalization of the "Backs" up there is another potential source of error.* Untested too ;) *anyone else notice something I'm missing in this code?* No you're, not missing anything. You should be able to use an event-driven strategy in your own code.
Juliet
Yet I'm still not getting into the `if (OnHookCallback != null)` statement... how do I make that non-null? Commenting out the if statement results in a `'System.NullReferenceException'` and my event code does not trigger.
cksubs
I thought that's what the `MessageHooks.OnHookCallBack += MessageHooks_OnHookCallBack;` code did, btw, got me into that `if (OnHookCallback != null) statement. Is that not true, or is that statement for some reason not applying?
cksubs
@cksubs: see the documentation on C# events: http://msdn.microsoft.com/en-us/library/ms366768.aspx . A null event handler means there's no subscribers, non-null means there's a subscriber, so you *always* have to check to see if your event handlers are null before invoking them, otherwise you'll get a null reference exception. `obj.Event += someFunc` adds a subscriber, `obj.Event -= someFunc` removes a subscriber. The code I've written is correct and idiomatic. If you app still isn't working, could you paste the changes you've made to http://www.pastebin.com
Juliet
Sure, thank you for taking a look. I believe this includes all relevant code: http://pastebin.com/m5e00f46c
cksubs
I've reviewed the code and even created a test project, everything works fine on my end, so you either have a bug in your code or you aren't testing it correctly. If your event handler is returning null, it means there's no subscriber -- which, with very high likelihood, implies that you either haven't created or you created a Form1 instance and immediately closed an instance of your form before testing your method. At this point, there's nothing more I can add. Step through your code with your debugger.
Juliet
Would you mind sending me your test project? cksubs at gmail. I don't understand this. Mine should work perfectly. Thank you for all your help.
cksubs
Got it. Thank you so much.
cksubs
Nice solution, thought he could not change excisting code. :). Although +1 is fairly low for your hard work I cannot give you more.
bastijn