"That's not how you put a function inside a function"

My Girlfriend looked at me and said, “You’re not doing it right!”

So I showed it to her:

function get_html_translation ($html_translation_id, $language_id) {
	$query = "SELECT html_text FROM html WHERE html_translation_id = '$html_translation_id' AND language_id = '$language_id'";
	$result = mysql_query($query);
	$rows = mysql_num_rows($result);
	if ($rows > 0) {
		while ($array = mysql_fetch_assoc($result)) {
			$html = $array['html_text'];
		}
	}
	else {
		$query = "SELECT html_text FROM html WHERE html_translation_id = '$html_translation_id' AND language_id = '1'";
		$result = mysql_query($query);
		if ($rows > 0) {
			while ($array = mysql_fetch_assoc($result)) {
				$html = $array['html_text'];
			}
		}
		else {
			$html = '<p>[no_data]</p>';
		}
	}
	return $html;
}

function popup() {
	$html = get_html_translation($html_translation_id, $language_id);
	$popup = '<a href="#" id="pop" >PopUp</a><br />
	<div id="overlay_form" style="display:none">'
	.$html.'
	<a href="#" id="close" >Close</a>
	</div>';
	return $popup;
}


$language_id = '1';
$html_translation_id = '1';
$popup = popup();
/*
$html = get_html_translation($html_translation_id, $language_id);
	$popup = '<div id="overlay_form" style="display:none">'
	.$html.'
	<a href="#" id="close" >Close</a></div>';
	
*/

$translation = get_html_translation ($html_translation_id, $language_id);
$output = $output.$translation.$popup;


“That’s not how you put a function inside a function”. - she said.

Help me restore my self worth.

How do you put a function inside a function?

You can do it that way. It’d be better if it were in classes really, but I don’t see anything overtly wrong with that code?

Having said that, you shouldn’t be echoing html from within your functions. Your functions should just have logic in them, and the html should be outside of the functions. It’ll make your code more flexible and easier to change in the future.

Can you provide more clarity? What exactly do you mean a “function inside a function”?

For example, there are a few issues with your code, popup will always use “” for $html_translation_id and $language_id for one, unless you pass them to that function or declare them global, but I don’t think that is what you are asking for…


// give lang id a default value

function get_html_translation ($html_translation_id, $language_id = 1) {

// set the default value for html here ....
            $html = '<p>[no_data]</p>'; 


// if lang id is not set, it is now set to 1
 
    $query = "SELECT html_text FROM html WHERE html_translation_id = '$html_translation_id' AND language_id = '$language_id'"; 
    $result = mysql_query($query); 
    $rows = mysql_num_rows($result); 

// do you really have more than one result, ever?
// if so, you only ever get the last one ....

    if ($rows > 0) { 
        while ($array = mysql_fetch_assoc($result)) { 
            $html = $array['html_text']; 
        } 
    } 

// if that lot was NOT instantiated, then your default html value will fall through to here ...

    return $html; 
}

She’s a bit right.

From the two lines “return $popup” and “$popup = popup()”, I get the impression the OP is attempting to use inline / anonymous methods. In most cases, the inline method is defined within the scope of the calling method, not outside. The reason of course is to keep scope local to the calling function. Having it outside the calling function is pointless. You may as well make it a regular function. Here’s a good page on the subject.

http://www.os-cms.net/blog/view/28/anonymous-and-inline-functions-with-php

Here is an example of what I am trying to do.

$language_id = '1';
$html_translation_id = '1';
$popup = popup();
$translation = get_html_translation ($html_translation_id, $language_id);
$output = $output.$translation.$popup;

I know the get_html_translation function works because the contents are output.

however the function popup doesn’t seem to be able use the get_html_translation function

The function get_html_translation does not work inside the function popup

http://www.spanishproperty.es/index.php?get_action=popup

That’s because the parameters that popup passes to get_html_translation are always going to be blank. (cpradio already mentioned that a couple days ago, btw.) If popup is going to use global variables (which is a bad idea, but…) then those variables need to be declared with the global keyword.