Alphabet Navigation Bar thing

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
User avatar
daedalus__
DevNet Resident
Posts: 1925
Joined: Thu Feb 09, 2006 4:52 pm

Alphabet Navigation Bar thing

Post by daedalus__ »

I wrote this today because I had to do an alphabet navigation thing for a companies' website that i maintain.

I'm sure others have been curious about this so I thought I would post it.

This class takes a table name and a column producing an alphabet navigation bar that links letters where there are records that begin with that letter. <- bad grammar

I don't work particularly hard for this company so I am sure that this class could see a lot of improvement.

Code: Select all

class AbcBar
{
	public $Chars = array();
	public $Alphabet = array();
	
	public $TableName;
	public $ColumnName;

	public function __construct($table_name, $column_name)
	{
		$this->TableName = $table_name;
		$this->ColumnName = $column_name;
		$this->GetChars();
		$this->PopulateAlphabetArray();
	}

	private function GetChars()
	{
		$sql = "SELECT DISTINCT LEFT(`{$this->ColumnName}`, 1) as first_letter FROM `{$this->TableName}`";
		$result = mysql_query($sql);
		while ($row = mysql_fetch_row($result))
		{
			$this->Chars[] = strtoupper($row[0]);
		}
	}

	private function PopulateAlphabetArray()
	{
		$this->Alphabet = range("a", "z");
	}

	public function CreateAlphabetNavigationBar()
        {
                foreach ($this->Alphabet as $letter)
                {
                        if (in_array(letter, $this->Chars))
                        {
                                $navi[] = '<a href="?letter='.$letter.'" title="Sort links by the letter '.$letter.'">'.$letter.'</a>';
                        }
                        else
                        {
                                $navi[] = $letter;
                        }
                }
                echo implode(" | ", $navi);
        }
}

mysql_connect('localhost', 'adfasdfasdfasdfasdfasdffasdffasdfasdf', 'safsdfadsdfasdfasdfasfdas');
mysql_select_db('test');

$abc = new AbcBar('links', 'name');

$abc->CreateAlphabetNavigationBar();

EDIT: Made one small improvement, removing the RemoveDuplicateChars() function, which made sure the array only contained unique chars, in favor of using the SQL DISTINCT keyword.

EDIT2: Added JayBird's suggestions. :D
Last edited by daedalus__ on Wed Feb 28, 2007 3:33 pm, edited 3 times in total.
User avatar
JayBird
Admin
Posts: 4524
Joined: Wed Aug 13, 2003 7:02 am
Location: York, UK
Contact:

Post by JayBird »

Instead of

Code: Select all

private function PopulateAlphabetArray()
        {
                for ($i = 65; $i <= 90; $i++)
                {
                        $this->Alphabet[] = chr($i);
                }
        }
You could just do

Code: Select all

private function PopulateAlphabetArray()
        {
                $this->Alphabet = range("a", "z");
        }
User avatar
JayBird
Admin
Posts: 4524
Joined: Wed Aug 13, 2003 7:02 am
Location: York, UK
Contact:

Post by JayBird »

And for this

Code: Select all

public function CreateAlphabetNavigationBar()
        {
                for ($i = 0; $i < 26; $i++)
                {
                        if (in_array($this->Alphabet[$i], $this->Chars))
                        {
                                echo '<a href="?letter='.$this->Alphabet[$i].'" title="Sort links by the letter '.$this->Alphabet[$i].'">';
                                echo $this->Alphabet[$i];
                                echo '</a>';
                        }
                        else
                        {
                                echo $this->Alphabet[$i];
                        }
                        if ($i < 25)
                        {
                                echo ' | ';
                        }
                }
        }
i would do something like

Code: Select all

public function CreateAlphabetNavigationBar()
{
	foreach ($this->Alphabet as $letter)
	{
		if (in_array(letter, $this->Chars))
			$navi[] = '<a href="?letter='.$letter.'" title="Sort links by the letter '.$letter.'">'.$letter.'</a>';
		else
			$navi[] = $letter;
	}
	echo implode(" | ", $navi);
}
User avatar
daedalus__
DevNet Resident
Posts: 1925
Joined: Thu Feb 09, 2006 4:52 pm

Post by daedalus__ »

Thank you, JayBird. Those are very nice improvements. Added. :D
User avatar
JayBird
Admin
Posts: 4524
Joined: Wed Aug 13, 2003 7:02 am
Location: York, UK
Contact:

Post by JayBird »

Just thinking you could also add numbers to the list.

Your navigation could look like this

# | a | b | c | d ...
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

IMO classes have very little business echo'ing out things... perhaps try returning the output.

Code: Select all

public function CreateAlphabetNavigationBar()
{
        /**
          * snip
        */
        return implode(" | ", $navi);
}
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

I agree with Jcart. use the methods to return data, then echo them or parse them into your templater.
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

Jcart wrote:IMO classes have very little business echo'ing out things... perhaps try returning the output.
What if it's something like:

Code: Select all

class View
{
    public function output($input, $return = false)
    {
        $output = htmlentities($input);

        // should i...
        return $output;

        // or maybe...
        echo $output;

        // possibly even give user the option?
        if ($return) return $output;
        else return $output
    }
}
serious question
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

I have done that before, but only in circumstances where there is no chance of being able to parse a return value into the template. The one exception to this is when I build a utility that dumps data, in which case echoing is almost a requirement.
User avatar
daedalus__
DevNet Resident
Posts: 1925
Joined: Thu Feb 09, 2006 4:52 pm

Post by daedalus__ »

Jcart wrote:IMO classes have very little business echo'ing out things... perhaps try returning the output.
daedalus__ wrote:I don't work particularly hard for this company so I am sure that this class could see a lot of improvement.
I thought that, the way it is, is fine for demonstrating how to do this. If this was for myself or a higher paying company, I would not echo anything. I can change it if you guys think I should, though. My goal is to simply demonstrate one way to create an alphabet navigation thing.

I'll change it to support numbers later tonight.
Post Reply