[RESOLVED] 4pm & my $brain_hurts ... function to eliminate object code?

[dalecosp]

I'm cleaning up an "important" system I've been working on for a few years now.

In one script I keep finding this:

if (!$object->color) {

$object->color = getColor($object);
}
if (!$object->color) {

$object->color = getAltColor($object);
}

The two functions simply examine the object and return a string about the color of the object, which is assigned to the appropriate var. The script calls this routine many times, depending on branching of the logic and so forth.

Given that I've already got these two functions, how can I refactor and "wrap them up" so that I can eliminate repeating this code block over and over? It seems it ought to be simple, but my noggin' is whacked out right now from a day full of questions from the design & content people (and the supervisor) and a few hours of writing technical documentation ... bah. :queasy:

[cluelessPHP]

Not tested but can't you do something like?

	function IgetColours()
	{
			if (!$object->color) { 

			$object->color = getColor($object) || $object->color = getAltColor($object); 
		} 
}	

 IgetColours();

23:47 here I wins? I'm sure I'll look at this in the morning and groan, on the bright side I do sit near an open window these days

[NogDog]

Because of operator precedence, you'd either need to do:

($object->color = getColor($object)) || ($object->color = getAltColor($object));
//...or...//
$object->color = getColor($object) or $object->color = getAltColor($object);

[cluelessPHP]

NogDog;11060111 wrote:

Because of operator precedence, you'd either need to do:

($object->color = getColor($object)) || ($object->color = getAltColor($object));

I guess, although I guess I misread it I thought he's have something like

function func1(){

//something

}

func2{
//something
}

Then this would all be on one page, that's been a personal habit recently could be wrong I guess :o

[dalecosp]

cluelessPHP;11060109 wrote:

Not tested but can't you do something like?

	function IgetColours()
	{
			if (!$object->color) { 

			$object->color = getColor($object) || $object->color = getAltColor($object); 
		} 
}	

 IgetColours();

23:47 here I wins? I'm sure I'll look at this in the morning and groan, on the bright side I do sit near an open window these days

Well, you can have today's "Selfless Devotion" award 😃

I'll take a look at this idea, but for starters, I think your IGetColours() needs to have the object passed into it? (And then there's that matter of "speling" ... :p)

NogDog;11060111 wrote:

Because of operator precedence, you'd either need to do:

($object->color = getColor($object)) || ($object->color = getAltColor($object));
//...or...//
$object->color = getColor($object) or $object->color = getAltColor($object);

I guess that would shorten it up a tad ... I'm really hoping for a one-liner. Suppose I'll write a test script today; system's too important to risk breaking the whole thing ...

[Derokorian]

dalecosp;11060121 wrote:

I guess that would shorten it up a tad ... I'm really hoping for a one-liner. Suppose I'll write a test script today; system's too important to risk breaking the whole thing ...

Those are one liners. Also if you really want a one liner, make getColor call getAltColor if need be, so you just have to do $obj->color = getColor($object). To that end, if you're passing in the object to a function just to set a property on that object, why not make an object function, called setColor() that does this inside of it. This way you never have to call these funcs again, you just call $obj->setColor() wherever you want to make sure its set.

[NogDog]

[EDIT]Added missing [FONT=Courier New]$object->color[/FONT] =[/EDIT]

While I sometimes shun one-liners as making things less readable at times....

$object->color = $object->color or $object->color = getColor($object) or $object->color = getAltColor($object);

(I think?)

[cluelessPHP]

function IgetColors() //fixed for colonial tax evaders spelling :p
{
	require('GetColor.php'); //or whatever path it should be 

	$object = new GetColor();

		if (!$object->color) 
		{  
			$object->color = getColor($object) || $object->color = getAltColor($object); 
		} 
}	

echo IgetColors();

Meh you can call a class in a function too or so google tells me, I could be completely wrong the other two are miles ahead of me as you know :p

[dalecosp]

Derokorian wrote:

why not make an object function, called setColor() that does this inside of it.

Yes, that's the "Right Thing", I suppose. Alas, this part of the system's not as object-oriented as some other parts are. I don't want to have to integrate both functions into the object class today; it's a tad too large already, anyway (1003 LOC), and feels a lot like a kitchen sink.

function colorCheck($obj) {
   if (strlen($obj->color)) return;
   $obj->color = MachineColor($obj);
   if (strlen($obj->color)) return;
   $obj->color = getColor($obj);
   return;
}

//Old usage
if (!$object->color) { 
    $object->color = getColor($object); 
} 
if (!$object->color) {          

    $object->color = getAltColor($object); 
}  

// new usage
colorCheck($object);

I think I was mostly concerned/confused about scoping issues, and whether the function would operate on the actual object. Testing shows this will work for the time being. Thanks to all of you fine gentlemen for helping the old man out. 🙂

[Derokorian]

dalecosp;11060143 wrote:

Yes, that's the "Right Thing", I suppose. Alas, this part of the system's not as object-oriented as some other parts are. I don't want to have to integrate both functions into the object class today; it's a tad too large already, anyway (1003 LOC), and feels a lot like a kitchen sink.

Sounds like it might be time for some traits and/or inheritance 😛

dalecosp;11060143 wrote:

I think I was mostly concerned/confused about scoping issues, and whether the function would operate on the actual object. Testing shows this will work for the time being. Thanks to all of you fine gentlemen for helping the old man out. 🙂

Objects are always passed by reference. If it wasn't you could always change the function signature to take it by reference.

[dalecosp]

Derokorian;11060159 wrote:

change

The word can strike fear into the stoutest heart 😉

Sounds like it might be time for some traits and/or inheritance :p

Yeah, if I actually had some actual & realistic mental picture of how {a brain*} works, I'd probably already have some inheritance, some kind of OO paradigm. I just barely got the thing cobbled together as it was.

*I say "a brain" ... it is a thinking process, I suppose, not nearly as complex as a brain, but I had no clue about how to do what it does in an "orderly fashion" ...

[Derokorian]

dalecosp;11060169 wrote:

Yeah, if I actually had some actual & realistic mental picture of how {a brain*} works, I'd probably already have some inheritance, some kind of OO paradigm. I just barely got the thing cobbled together as it was.

I would help with that, but the only thing I know about your class is that it has a color property.

[dalecosp]

Derokorian;11060175 wrote:

I would help with that, but the only thing I know about your class is that it has a color property.

Not sure what I'm allowed to say 😉

www.ombe.com/ezlink

It's a four-stage system. 1, get data from someplace, 2, figure out what it is, 3, run some custom modification rules against it, 4, put it on our web site.

Step 2, in my mind, resembles some AI ... it's an attempt to emulate the process a Real Person(tm) would undertake to decide if some product is a Color MFP or a Cyan Toner Cartridge or a Fuser Roller Bearing, or ... whatever it really is. I wasn't sure how to OO this.

I currently estimate it has a 90+% accuracy rating. Of course, with 150K products, that means I've got 15,000 mis-categorized pieces. I'm always looking to improve it, but it's in use daily ... fortunately, I DO have a sandbox for this one, unlike some of our projects 😉

The latest "improvement" to the overall system is Spell checking. Needless to say, it helps step 2 do its work if the words in the descriptions, titles, etc., are spelled correctly. 🙂

Not a lot of time to spare to improve it, though. Everyone else thinks it works well enough, I guess ... or, in reality, they're too busy to care. Probably I am too ... but I do care, at least a little. I saw these conditionals all over and thought "I should clean THAT up". And you all did help with that, thanks!!!

Not sure I really should prevail on you gentlefolk to do ALL of my work for me, though 😉 😉