[comp.lang.objective-c] Object Oriented Design:Scenario:Opinions Sought

chandra@bhairavi.uucp (B. Chandramouli) (12/27/90)

This is meant for the philosophically inclined :=)

I guess many of you would have run into the following situation. 

Scenario: (This is a little bit long but I promise to keep it as easy
	   reading as possible).

A class has two instance variables.  Let us call them INV1 and INV2.

The main objective of the instances of this class are to take the value from
INV1 and put it on a screen location identified by INV2.
This method to accomplish this simple task looks like this (Pardon the
Smalltalk/Objective-C ish notation) :

	-refresh {
	  [userInterfaceManager	at:INV2 put:INV1];
	}

(userInterfaceManager is the all capable UIMS)

If there are 20 such screen locations, there will be 20 instances of 
this class.

The program is working fine but one fine morning the requirements changed.

	"For just one screen location X, if the value is "PANIC", then 
	in addition to putting up the value, display it in reverse video."

There are two choices:

1) Create a subclass and override "refresh" and just create one instance
   of this class to manage Screen Location X.

	-refresh {
	  if (INV1 == "PANIC") {
	  	[userInterfaceManager	at:INV2 put:INV1];
	  	[userInterfaceManager at:INV2 action:REVERSEVIDEO];
	  }
	}

2) Or, modify the refresh of the existing class as follows, such that
   the new behavior applies only for that screen location (i.e exectued
   only by the instance responsible for that screen location).

	-refresh {

	   // Check if this instance controls screen location X
	   if (INV2 == X)  {
	  	if (INV1 == "PANIC") {
	  		[userInterfaceManager	at:INV2 put:INV1];
	  		[userInterfaceManager at:INV2 action:REVERSEVIDEO];
	  	}
	   }
	}

Now my questions:

1) Which is better and why?

2) Which is more object oriented?

Thanks.

chandra

wiml@milton.u.washington.edu (William Lewis) (12/27/90)

In article <3302@mrsvr.UUCP> chandra@bhairavi.uucp (B. Chandramouli) writes:
>	"For just one screen location X, if the value is "PANIC", then 
>	in addition to putting up the value, display it in reverse video."

>1) Create a subclass and override "refresh" and just create one instance
>   of this class to manage Screen Location X.
>
>	-refresh {
>	  if (INV1 == "PANIC") {
>	  	[userInterfaceManager	at:INV2 put:INV1];
>	  	[userInterfaceManager at:INV2 action:REVERSEVIDEO];
>	  }
>	}

>2) Or, modify the refresh of the existing class as follows, such that
>   the new behavior applies only for that screen location (i.e exectued
>   only by the instance responsible for that screen location).

>1) Which is better and why?
>2) Which is more object oriented?

   Well, I don't now if I'm about to answer those two questions ... but
the way I, personally, would do it would be to create a subclass that
checks to see if its value is 'PANIC' and displays it in inverse 
video. Actually, I'd probably add a third instance variable to that class,
such that if INV3 == INV2, it's put in reverse video. If the code allows
INV3 to equal NULL (and never reverse-videos the field in this case),
then the subclass does everything the superclass does and more, and
the superclass can be discarded; equivalently, this is just modifying ths
superclass. (INV3 should be set to NULL in the +new method, for compatibility
with the old class). Why would I do this? ... Well, it seems to me that
"if string = "PANIC" and location = arbitrary-place, then use
inverse video" is a pretty arbitrary condition. Also, the ability
to invert itself under certain conditions seems to me like a quality
of the field on the screen, not of the string ("PANIC" is not inherently
inverse video) or of the location (if you rearranged your display a little
and the field got shifted 1 pixel, you'd still want it to act the same).
So the attribute that causes inversion should be an attribute _of_the_
_field_. And from there it doesn't hurt to generalize to inverting 
strings other than "PANIC".
  This also seems more object-oriented: it's using the field as an 
object which has certain qualities (such as inversion) independent
of other qualities (such as location, or string value). And the generality
makes it easy to reuse the same code in multiple places and multiple
applications. 

  (stows soapbox back under desk)   Well, that's my opinion ...



-- 
 wiml@milton.acs.washington.edu       Seattle, Washington   
     (William Lewis)   |  47 41' 15" N   122 42' 58" W  
"These 2 cents will cost the net thousands upon thousands of 
dollars to send everywhere. Are you sure you want to do this?"

dbrenner@icon.weeg.uiowa.edu (Doug Brenner) (12/27/90)

In article <3302@mrsvr.UUCP> chandra@bhairavi.uucp (B. Chandramouli) writes:
> There are two choices:
>  1) Create a subclass and override "refresh" and just create one instance
>     of this class to manage Screen Location X.
>     [...code deleted...]
>  2) Or, modify the refresh of the existing class as follows, such that
>     the new behavior applies only for that screen location (i.e exectued
>     only by the instance responsible for that screen location).
>     [...code deleted...]
>
> Now my questions:
>  1) Which is better and why?
>  2) Which is more object oriented?

I hope I've understood your question completely.  Here goes.  First some
questions of my own (and then my answers).

If you pick choice 1 (subclass):
  o Are you *really* creating a new object?
  o What would happen if you did this everytime you needed to add
    functionality to an object?
  o If someone has already subclassed the original object AND they want
    to utilize this new "highlight" functionality, what do THEY do?

If you pick choice 2 (same class, new code):
  o If someone is using the existing class and doesn't want this new
    functionality, what do THEY do?

To try and answer, I would suggest NOT subclassing.  Rather, add new
functionality in a way that makes things upward compatible.

First, I'd say you really don't have a new object, just the same old
object with some new functionality.  Creating a subclass is overkill.

How about this code (written in Objectionable-C):

  BOOL highlight = NO;        /* YES for highlighting; NO for none */
  char *highlightStr = "";    /* string to highlight if seen */

- setHighlight: (BOOL) theHighlight
{
   highlight = theHighlight;
   return self;
}

- setHighlightStr: (const char *) theHighlightStr
{
   /* malloc and strcpy theHighlightStr (an exercise for the reader :-) */
   return self;
}

-refresh
{
   [userInterfaceManager at:INV2 put:INV1];
   if (highlight == YES && INV1 == highlightStr)
      [userInterfaceManager at:INV2 action:REVERSEVIDEO];
   return self;
}

This allows those who used the old class, either directly or via subclassing,
to retain their old functionality.  It also allows those same people to gain
access to this new functionality should they wish.

You, who need the new functionality, must enable it.  (You expected to do
this WITHOUT code changes?  Sorry, you're in the wrong field!  :-) :-)

You must perform two additional steps each time you create the object and you
want it to highlight.

   myObj = [Obj new];
   [myObj setHighlightStr:"PANIC"];  /* set the string to key on */
   [myObj setHighlight:YES];         /* turn on highlighting */

Now to make your life easier, you might consider adding another factory
method that will do this in one step.  Still code changes, but you might
find this to your liking.  (The previous code is still required.)

+ newHighlight: (BOOL) flag highlightStr: (const char *) str;
{
   self = [super new];
   /* malloc and strcpy str */
   highlight = flag;
   return self;
}

Then you would just:

   myObj = [Obj newHighlight:YES highlightStr:"PANIC"];

(I will not agrue if this type of "paramater burden" is a good idea for
factory methods.  The NeXT AppKit is full of such examples, however.)

Good luck and remember...these are just *my* opinions.  -dcb

lerman@stpstn.UUCP (Ken Lerman) (12/28/90)

In article <3302@mrsvr.UUCP. chandra@bhairavi.uucp (B. Chandramouli) writes:
.This is meant for the philosophically inclined :=)
.
.I guess many of you would have run into the following situation. 
.
.Scenario: (This is a little bit long but I promise to keep it as easy
.	   reading as possible).
.
.A class has two instance variables.  Let us call them INV1 and INV2.
.
.The main objective of the instances of this class are to take the value from
.INV1 and put it on a screen location identified by INV2.
.This method to accomplish this simple task looks like this (Pardon the
.Smalltalk/Objective-C ish notation) :
.
.	-refresh {
.	  [userInterfaceManager	at:INV2 put:INV1];
.	}
.
.(userInterfaceManager is the all capable UIMS)
.
.If there are 20 such screen locations, there will be 20 instances of 
.this class.
.
.The program is working fine but one fine morning the requirements changed.
.
.	"For just one screen location X, if the value is "PANIC", then 
.	in addition to putting up the value, display it in reverse video."
.
.There are two choices:
.
.1) Create a subclass and override "refresh" and just create one instance
.   of this class to manage Screen Location X.
.
.	-refresh {
.	  if (INV1 == "PANIC") {
.	  	[userInterfaceManager	at:INV2 put:INV1];
.	  	[userInterfaceManager at:INV2 action:REVERSEVIDEO];
.	  }
.	}
.
.2) Or, modify the refresh of the existing class as follows, such that
.   the new behavior applies only for that screen location (i.e exectued
.   only by the instance responsible for that screen location).
.
.	-refresh {
.
.	   // Check if this instance controls screen location X
.	   if (INV2 == X)  {
.	  	if (INV1 == "PANIC") {
.	  		[userInterfaceManager	at:INV2 put:INV1];
.	  		[userInterfaceManager at:INV2 action:REVERSEVIDEO];
.	  	}
.	   }
.	}
.
.Now my questions:
.
.1) Which is better and why?
.
.2) Which is more object oriented?
.
.Thanks.
.
.chandra


Alternative 3)

Subclass adding a new instance variable: panicLocation

-refresh
{
	if(INV2 == panicLocation)
	{
		[userInterfaceManager	at:INV2 put:INV1];
  		[userInterfaceManager at:INV2 action:REVERSEVIDEO];
	}
}

Then when an instance is created set the panicLocation appropriatly.
This lets you have multiple panic locations.

I don't like the second solution because it creates a class which is
too specific for reuse.  The second solution is not general enough.

All of the should be liberally sprinkled with IMHOs.  The specific
context can be very important.  As can the schedule and the present
state of affairs. :-)

Ken