views:

501

answers:

6

i want to mark a method as obsolete, but Delphi 5 doesn't have such a feature.

For the sake of an example, here is a made-up method with it's deprecated and new preferred form:

procedure TStormPeaksQuest.BlowHodirsHorn; overload; //obsolete
procedure TStormPeaksQuest.BlowHodirsHorn(UseProtection: Boolean); overload;

Note: For this hypothetical example, we assume that using the parameterless version is just plain bad. There are problems with not "using protection" - which have no good solution. Nobody likes having to use protection, but nobody wants to not use protection. So we make the caller decide if they want to use protection or not when blowing Hodir's horn. If we default the parameterless version to continue not using protection:

procedure TStormPeaksQuest.BlowHodirsHorn;
begin
    BlowHodirsHorn(False); //No protection. Bad!
end;

then the developer is at risk of all kinds of nasty stuff. If we force the parameterless version to use protection:

procedure TStormPeaksQuest.BlowHodirsHorn;
begin
    BlowHodirsHorn(True); //Use protection; crash if there isn't any
end;

then there's a potential for problems if the developer didn't get any protection, or doesn't own any.

Now i could rename the obsolete method:

procedure TStormPeaksQuest.BlowHodirsHorn_Deprecatedd; overload; //obsolete
procedure TStormPeaksQuest.BlowHodirsHorn(UseProtection: Boolean); overload;

But that will cause a compile error, and people will bitch at me (and i really don't want to hear their whining). i want them to get a nag, rather than an actual error.

i thought about adding an assertion:

procedure TStormPeaksQuest.BlowHodirsHorn; //obsolete
begin
   Assert(false, 'TStormPeaksQuest.BlowHodirsHorn is deprecated. Use BlowHodirsHorn(Boolean)');

   ...
end;

But i cannot guarantee that the developer won't ship a version without assertions, causing a nasty crash for the customer.

i thought about using only throwing an assertion if the developer is debugging:

procedure TStormPeaksQuest.BlowHodirsHorn; //obsolete
begin
   if DebugHook > 0 then
      Assert(false, 'TStormPeaksQuest.BlowHodirsHorn is deprecated. Use BlowHodirsHorn(Boolean)');

   ...
end;

But i really don't want to be causing a crash at all.

i thought of showing a MessageDlg if they're in the debugger (which is a technique i've done in the past):

procedure TStormPeaksQuest.BlowHodirsHorn; //obsolete
begin
   if DebugHook > 0 then
        MessageDlg('TStormPeaksQuest.BlowHodirsHorn is deprecated. Use BlowHodirsHorn(Boolean)', mtWarning, [mbOk], 0);

   ...
end;

but that is still too disruptive. And it has caused problems where the code is stuck at showing a modal dialog, but the dialog box wasn't obviously visible.

i was hoping for some sort of warning message that will sit there nagging them - until they gouge their eyes out and finally change their code.

i thought perhaps if i added an unused variable:

procedure TStormPeaksQuest.BlowHodirsHorn; //obsolete
var
   ThisMethodIsObsolete: Boolean;
begin
   ...
end;

i was hoping this would cause a hint only if someone referenced the code. But Delphi shows a hint even if you don't call actually use the obsolete method.

Can anyone think of anything else?

A: 

Why do you want to do this and why don't you want to upgrade the Delphi version?

Without the deprecated tag you really have no clean option to filter the use of deprecated methods. So it depend on where you want to make the concession:

  • renaming catches the error at compiletime (unless there is another method/function with the same name within scope).
  • all other methods are only caught at runtime. And this has a risk of slipping into the production code.

What you can do, is create a deprecated log. This won't piss off anybody, and it is no complete disaster if it enters production code. But if your tests have full coverage, you will catch all the culprits. You only have to check the log file after a (test)run.

And of course the best way, is to use grep to find all the occurences of the code and change it.

Gamecat
i have thought about adding OutputDebugString(), so that nobody is bothered - but nobody will actually ever see it. And without the eye-sore of a warning sitting there, nobody will fix it.
Ian Boyd
Also i don't want to upgrade the Delphi version because i don't happen to have a few tens of thousands of dollars laying around.
Ian Boyd
+2  A: 

This doesn't exactly answer your question but it might provide an alternative solution. Could you not update the original function with a default value...

procedure TStormPeaksQuest.BlowHaldirsHorn(UseProtection: Boolean = False);

...so that legacy code compiles and behaves the same but the new functionality is available to new developers.

This approach is the most desirable for a commercial product or when you don't want to break a large amount of existing code.
skamradt
The problem is what do i default it to? Do i default it to mirror the current (incorrect) behavior, in which case nothing is fixed? Or do i default it to use the different (read: new better ideal) behaviour, in which case they ship it untested, and it crashes.
Ian Boyd
And, btw, default values are identical to overloads. Except in this case i don't want people thinking that it's okay to use the default values. The proper solution is to fix your code, and opt into the new system. And removing the reference to the old method is how you opt into the new method.
Ian Boyd
A: 

I'd agree with an optional parameter if it'll do the job. But I can think of situations were optional params won't fit. For instance I have moved functions into new units but kept the olds ones and marked them as deprecated.

I guess whatever solution you go with will also depend on the discipline of your team. Do they actively take notice and work to correct all the hints and warnings for their apps? Hopefully they do but I am shamed to admit that the team I work with (including myself) do not stay on top of all the hints and warnings. Every now and then I fix as many hints & warnings as time permits and we absolutely should fix warnings but in reality we've got to get the job done and are more focused on new features and deadlines.

So my point is, even if you could mark them as deprecated or give a similar hint/warning, do you feel your team will take the time to change their code anyway?

Ben Daniel
We are meticulous (sp?) about hints and warnings; it's an eye-sore that nobody likes to see.
Ian Boyd
+4  A: 

How about something like

procedure TStormPeaksQuest.BlowHaldirsHorn; //obsolete
begin
  if DebugHook > 0 then asm int 3 end;  
  // This method is Obsolete!  Use XXXXX instead.
  Abort; // Optional, makes method useless  
  // old code here . . . 
end;

Kind of a compromise between an assertion and a showmessage. The developer just needs to hit F9 to continue. You could put in an Abort, and then the method would do nothing, and that would force them to switch methods and the break makes them aware of it.

Personally I would recommend upgrading to a newer version of Delphi. 2007 and 2009 are great releases and are really worth the upgrade.

Jim McKeeth
i thought about adding if DebugHook>0 then Windows.DebugBreak;, but that isn't obvious that anything is really wrong (some code shipped by MS left INT3's in there, so we are conditioned to hit F9 whenever the debugger randomly popped up).
Ian Boyd
i also wouldn't want to add an Abort, either at runtime, or in DebugHook only, since if there's an emergency update to a piece of software that nobody has looked at in 3 years: i don't want to be the one to have added random crashes to the software that nobody remembers how it works as it is.
Ian Boyd
+1  A: 

What i've ended up using was a combination of opting into a system where you agree to not have any depricated code, with output debug strings and breakpoints otherwise.

Like strict html, i've created a Strict define.

All the common units have the obsolete code defined out if Strict is defined. That way the developer has agreed that they will not have deprecated code in their project:

{$IFNDEF Strict}
procedure TStormPeaksQuest.BlowHaldirsHorn; overload; //obsolete
{$ENDIF}
procedure TStormPeaksQuest.BlowHaldirsHorn(UseProtection: Boolean); {$IFNDEF Strict}overload;{$ENDIF}


{$IFNDEF Strict}
procedure TStormPeaksQuest.BlowHaldirsHorn; //obsolete
begin
   OutputDebugString(PAnsiChar('TStormPeaksQuest.BlowHaldirsHorn is deprecated. Use BlowHaldirsHorn(Boolean)'));

   //Don't debugbreak without a debugger attached!
   if DebugHook > 0 then
        Windows.DebugBreak;

   ...
end;

So if the developer wants to have proper code, suffering having to perform code-changes when new things are deprecated, they can:

{$DEFINE Strict}

If not, then there will always be an OutputDebugString, and anyone with Debug View can see (even customers). It's funny to see commercial software (even Microsoft's) with output debug strings left over.

And finally, if there's a debugger attached, then they'll get a debug breakpoint out of nowhere. If anyone asks about, i can take that opportunity to make fun of them.

Ian Boyd
Interesting solution. If I were still at Delphi 5 I would probably steal it.
dummzeuch
A: 

It's not a total solution since you can't differentiate whether they've used the method or not but if it's available in Delphi 5 but you could use the $MESSAGE compiler directive to emit a warning at compile time.

LachlanG
$MESSAGE isn't available in D5.
Ian Boyd