1

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!

ck_
  • 3,719
  • 10
  • 49
  • 76
  • 1
    I think you are confusing AKA with i.e. lol – James Jan 15 '10 at 09:16
  • *Because the HookCallBack method is static, Form1.newKeyPress() needs to be static as well.* - **not true** . newKeyPress should be static isf you want to use Form1 class name as caller. But in general, man it's hard to understand what do you want... could you simplify/clarify your question. What is your target? – serhio Jan 15 '10 at 09:21
  • 2
    @cksubs...that really depends on where you are! It's morning over here lol – James Jan 15 '10 at 09:22
  • It has to be static unless I create a new form1 object, right? I don't want to do that, so it would thus have to be static? – ck_ Jan 15 '10 at 09:25
  • how is linked `InterceptKeys` with `HookCallback`? – serhio Jan 15 '10 at 09:27

4 Answers4

3

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
  • 80,494
  • 45
  • 196
  • 228
  • 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? – ck_ Jan 15 '10 at 09:58
  • @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 Jan 15 '10 at 10:25
  • @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 Jan 15 '10 at 10:53
2

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
  • 80,494
  • 45
  • 196
  • 228
  • 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 Jan 15 '10 at 10:45
  • 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? – ck_ Jan 15 '10 at 11:20
  • 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. – ck_ Jan 15 '10 at 11:58
  • I also changed all `MessageHooks` to `InterceptKeys` as that's what the class was previously named. – ck_ Jan 15 '10 at 12:01
  • Juliet must be sleeping... anyone else notice something I'm missing in this code? So close... – ck_ Jan 15 '10 at 17:37
  • *"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 Jan 15 '10 at 18:05
  • 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. – ck_ Jan 15 '10 at 21:02
  • 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? – ck_ Jan 15 '10 at 21:06
  • @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 Jan 15 '10 at 21:20
  • Sure, thank you for taking a look. I believe this includes all relevant code: http://pastebin.com/m5e00f46c – ck_ Jan 15 '10 at 21:42
  • 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 Jan 15 '10 at 22:33
  • 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. – ck_ Jan 15 '10 at 22:43
  • Nice solution, thought he could not change excisting code. :). Although +1 is fairly low for your hard work I cannot give you more. – bastijn Jan 16 '10 at 09:57
1

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
  • 62,551
  • 17
  • 151
  • 186
  • -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 Jan 15 '10 at 09:43
  • 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? – ck_ Jan 15 '10 at 09:47
1

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
  • 5,841
  • 5
  • 27
  • 43