wxSingleInstanceChecker as a one-liner

[wxtrac]

Issue migrated from trac ticket # 11166

component: base | priority: low | resolution: fixed | keywords: wxSingleInstanceChecker

2009-09-03 15:17:01: @troels-k created the issue

---

Why force the user to work out a mutex/lock name? S/he has better things to do, let the framework figure it out. Patch: Facilitate the simpler construct below

bool App::OnInit(void) { bool ok = wxApp::OnInit(); if (ok) { ok = m_checker.CreateAndCheck(); } return ok; }

[wxtrac]

2009-09-03 15:17:30: @troels-k uploaded file snglinst-28.patch (1.3 KiB)

2.8 branch

[wxtrac]

2009-09-03 15:20:08: @troels-k uploaded file snglinst.patch (1.8 KiB)

Trunk

[wxtrac]

2009-09-04 00:58:18: @vadz changed priority from normal to low

2009-09-04 00:58:18: @vadz changed status from new to infoneeded_new

2009-09-04 00:58:18: @vadz commented

I don't have anything against auto-generating the lock name but this really has nothing to do in appcmn.cpp, it should be in wxSingleInstanceChecker itself, maybe inline in wx/snglinst.h. Or maybe we could just auto-generate the name by default if ctor/Create() parameter is empty? OTOH people may expect the default ctor to do nothing and this wouldn't be true any more then. Maybe we could derive wxStdSingleInstanceChecker from the existing class which would do this?

I'm less sure about combining IsAnotherRunning() with Create() though. These functions just do different things and while it's true that you always call IsAnotherRunning() after Create() they're still different. I also don't know if it's a good idea to ignore the return code of Create() like this. If you really want to do it, just use non-default ctor and then you can call just the latter.

Finally I'd rather change this in the trunk only, I'm really not very kin on doing such minor enhancements in 2.8, this branch is in maintenance mode and only bug fixes should go into it (unless someone else is ready to take over maintaining it...).

[wxtrac]

2009-09-04 10:25:37: @troels-k changed status from infoneeded_new to new

2009-09-04 10:25:37: @troels-k commented

Hearing about the alternatives the patch as is looks like the lesser evil.

For reference, the objective is to free the user from carrying this block of cruft here

bool wxSingleInstanceCheck(wxSingleInstanceChecker* checker) {_ return checker->CreateAndCheck(); _wx2.9 ticket #11166 const wxString name = wxString::Format(wxT("%s-%s"), wxTheApp->GetAppName().wx_str(), wxGetUserId().wx_str()); return checker->Create(name) && !checker->IsAnotherRunning(); }

[wxtrac]

2009-09-13 16:36:47: @vadz changed status from new to infoneeded_new

2009-09-13 16:36:47: @vadz commented

Finally I think auto-generating the name if the corresponding parameter is empty is the best thing to do. We already do this with the path and I can't think of any real reason to not do it for the name too.

The issue with default constructor doing something could be avoided if we did this auto-generation in IsAnotherRunning() itself. I.e.:

_ (bad but some people will still do it like this...)
wxSingleInstanceChecker g_checker; // nothing done in ctor

bool MyApp::OnInit()
{
   // we could do this explicitly:
   // g_checker.Create("my unique name");

   // but as we hadn't initialized it before, it will be done using the
   // default name now
   if ( g_checker->IsAnotherRunning() )
      return false;
   ...
}

This also takes care of my philosophical objections to CreateAndCheck().

What do you think? If you don't see any problems with this approach, could you please update your patch to do it like this?

TIA!

[wxtrac]

2009-09-14 12:14:14: @troels-k changed status from infoneeded_new to new

2009-09-14 12:14:14: @troels-k commented

Nice idea. What to do about const? IsAnotherRunning is const, invoking non-const Create method from IsAnotherRunning is somewhat dirty.

[wxtrac]

2009-09-15 19:06:49: @vadz commented

---

I agree that it's somewhat dirty and if we designed this class now I'd make IsAnotherRunning() non-const because of it but as it is const now I think we should keep it const and use const_cast<> inside it.

After all, the object which creates itself on first use is a common idiom and not too bad IMO.

[wxtrac]

2009-09-15 22:09:41: @troels-k uploaded file snglinst2.patch (3.7 KiB)

Trunk

[wxtrac]

2009-09-15 22:10:40: @troels-k commented

---

Ok, fine. Added a new patch.

[wxtrac]

2009-09-16 14:38:03: @vadz changed status from new to closed

2009-09-16 14:38:03: @vadz changed resolution from * to fixed*

2009-09-16 14:38:03: @vadz commented

(In [61945]) Allow creating wxSingleInstanceChecker with default name.

This makes it easier to use in common cases: there is no need to come up with a unique name for the checker any more as sufficiently unique combination of wxApp::GetAppName() and wxGetUserId() is used if no name was explicitly given.

This is done by calling the new CreateDefault() on demand from IsAnotherRunning() instead of simply creating the checker with the default name in the default ctor for compatibility (you had to call Create() after using the default ctor before and it can only be called once) and because wxTheApp might not exist yet when wxSingleInstanceChecker is created.

Closes #11166.

[wxtrac]

2009-09-16 15:02:24: @troels-k commented

---

Thanks.

PS: My compatibility function for wx 2.8 users becomes

ifdef _WX_SNGLINSTH

inline bool wxSingleInstanceCheck(wxSingleInstanceChecker* checker) {

if (wxVERSION_NUMBER >= 2901)

return !checker->IsAnotherRunning();

else

const wxString name = wxString::Format(wxT("%s-%s"), wxTheApp->GetAppName().wx_str(), wxGetUserId().wx_str()); return checker->Create(name) && !checker->IsAnotherRunning();

endif

}

endif