Dodawanie danych do bazy przez formularz

0

Coś próbuję sklecić ale nie idzie mi to jakoś. Rzućcie proszę okiem na te kody z 2 plików.
Mam następujące problemy:
-gdy wyślę pusty formularz zadziała walidacja danych z metody validateInput() jednakże mimo tego rekord dodaje się do bazy danych:

error.png

-nie wiem jak przypisać dane z formularza do tego obiektu:

$ins->addNews('topic', 'news'); 

a konkretniej jak wstawić za topic i news odpowiednie dane.
Próbuję tak:

$ins->addNews($topic, $news); 

ale otrzymuję:

Notice: Undefined variable: topic in C:\xampp\htdocs\CMS\add_news.php on line 45
Próbuję tak

$ins->addNews($this->topic, $this->news); 

ale otrzymuję:

Fatal error: Using $this when not in object context in C:\xampp\htdocs\CMS\add_news.php on line 45

Klasa:

<?php
error_reporting(E_ALL);
ini_set("display_errors", 1); 

class ValidateUrl{
	private $_dbHandler;

    public function __construct($db){
        $this->_dbHandler = $db;
    }

    private $topic;
    private $news;
    public function filterTopic(){
    	$this->topic = $topic = isset($_POST['topic']) ? $_POST['topic'] : false;
   		$this->news = $news = isset($_POST['news']) ? $_POST['news'] : false;
   		return false;           
    }
    public function validateInput(){
    	if(empty($this->topic)) return print 'You must enter topic!';
    	else if(empty($this->news)) return print 'You must enter news!';
    	return false;
    }
    public function addNews($topic, $news){
		$add_date = date('Y-m-d H:i:s');
		$ip = $_SERVER['REMOTE_ADDR'];
		$host = gethostbyaddr($_SERVER['REMOTE_ADDR']);
		$nick = 'admin';
		$query = $this->_dbHandler->prepare("INSERT INTO `news` (`nick`, `date`, `topic`, `news`) VALUES (:nick, :add_date, :topic, :news)");
		$query->bindValue(":nick", $nick, PDO::PARAM_STR);
		$query->bindValue(":add_date", $add_date, PDO::PARAM_INT);
		$query->bindValue(":topic", $topic, PDO::PARAM_STR);
		$query->bindValue(":news", $news, PDO::PARAM_STR);
		if($query->execute() == false){	
			print_r($query->errorInfo());
			return false;		
			}else{	
				return true;
		}
	}	

}
?>

I wywoływanie klasy:

<!DOCTYPE html>
<html>
	<head>
		<meta charset="utf-8">
		<title>Add news - CMS</title>
		<style>
		a { color: #000; text-decoration:none; }
		a:hover { color: #40b000; }
		nav { text-align:center; width: 50%; }
		nav li { display: inline; list-style-type:none; }
		textarea { width: 250px; height: 150px; }
		.name { font-size: 1.250em; text-align: center; }
		.content { text-align:center; margin: 120px; }

		</style>
	</head>

	<body>
		<div class="name">
			<p>Content management system</p>
		</div>
		<nav>
			<ul>
				<li><a href="page">Home page</a></li>
				<li><a href="add_news">Add news</a></li>
			</ul>
		</nav>
		<div class="content">
			<form action="add_news" method="post">
				<p>Enter your news topic:</p>
					<input type="text" name="topic" placeholder="topic"/>
				<p>Enter your news</p>
					<textarea type="text" name="news" placeholder="news"></textarea>
				<p><button type="submit" name="submit_news">Add news</button></p>
			</form>
		</div>
		<?php
			require_once 'sql_database.class.php';
			require_once 'add_news.class.php';
			if(isset($_POST['submit_news'])){
				$dbObject = new SqlDatabase();
				$ins = new ValidateUrl($dbObject->getInstance());
				$ins->filterTopic();
				$ins->validateInput();
				$ins->addNews('topic', 'news'); 
			}
		?>
	</body>
</html>

Może ktoś poprawić mi mój kod? Męczę się z nim już trochę czasu i nie mogę tego zrobić. Próbuję się uczyć programowania obiektowego w PHP lecz jak na razie początki są ciężkie. Proszę o pomoc.

0

Na razie masz że masz zwalidować a potem dodać, nie sprecyzowałeś że chcesz dodać tylko wtedy gdy walidacja zostanie pomyślnie zakończona.

0

No właśnie, a co zrobić aby przysyłało dane gdy walidacja zostanie ukończona pomyślnie, że tak powiem?

0

Jeśli to klasa walidująca to powinna tylko walidować (można byłoby się pokusić o zbudowanie ładnego systemu modularnego, ale to raczej nie na starcie), a nie dodatkowo dodawać newsy czy robić inne dziwne rzeczy.
Dane powinieneś przekazywać w konstruktorze, a potem wywołać metodę odpowiadającą za sprawdzenie ich poprawności i na podstawie zwróconej wartości logicznej wysyłać zapytanie do bazy lub nie, jeśli są błędne.

0

Nie jest to klasa walidująca. Pewnie Cię zmyliło to ValidateUrl ale nie o to chodzi. Klasa powinna zawierać metody sotyczące filtracji, walidacji i dodawania rekordu do bazy. Czy mógłbyś nanieść poprawki na mój kod abby to zwizualizować? Bardzo bym prosił, pomoże mi to zrozumieć w SPACJA końcu mój problem.

0

Da radę mi ktoś to poprawić? Mogę zapłacić SMSem.

0

Klasa powinna zawierać metody sotyczące filtracji, walidacji i dodawania rekordu do bazy

Em, więc nie powinna nazywać się ValidateUrl.
Nie mówiąc o tym, że tym zdaniem dosłownie gwałcisz zasadę jednej odpowiedzialności.

Pomijając jednak tę zasadę (jako że to dział newbie i nie warto aż tak na początku mącić), wygodna do użytku będzie taka klasa:

<?php

class News {
	
	private $title, $content, $author;
	
	/**
	 * Dokonuje filtracji danych newsa.
	 * @return $this
	 */
	private function doFilter() {
		$this->title = trim($this->title);
		$this->content = trim($this->content);
		
		return $this;
	}
	
	/**
	 * Sprawdza poprawność newsa.
	 * @return bool
	 */
	public function isValid() {
		$this->doFilter();
		
		if (empty($this->title)) {
			return false;
		}
		
		if (empty($this->content)) {
			return false;
		}
		
		if (!ctype_digit($this->author)) {
			return false;
		}
		
		return true;
	}
	
	/**
	 * Zapisuje newsa do bazy danych.
	 * W razie niepowodzenia rzuca wyjątek, jeśli wszystko ok - zwraca `true`.
	 * @return bool
	 * @throws Exception
	 */
	public function doSave() {
		// komunikacja z bazą danych
		
		return true;
	}
	
	/**
	 * Ustawia tytuł newsa.
	 * @param string $title
	 * @return $this
	 */
	public function setTitle($title) {
		$this->title = $title;
		return $this;
	}
	
	/**
	 * Ustawia treść newsa.
	 * @param string $content
	 * @return $this
	 */
	public function setContent($content) {
		$this->content = $content;
		return $this;
	}
	
	/**
	 * Ustawia autora (id użytkownika).
	 * @param int $author
	 * @return $this
	 */
	public function setAuthorId($author) {
		$this->author = $author;
		return $this;
	}
	
	// @TODO przydałyby się gettery

}

I potem:

-- kod HTML --

<?php
if (isset($_POST['title'])) {
	$news = new News();

	// przypisz dane z formularza
	$news->
		setTitle($_POST['title'])->
		setContent($_POST['content'])->
		setAuthor(User::getUserId());
	
	// dokonaj walidacji newsa
	if ($news->isValid()) {
		// zapisz nius
		try {
			$news->doSave();
			echo 'News zapisany!';
		} catch (Exception $ex) {
			echo 'Wystąpił problem z zapisem newsa!';
		}
	} else {
		// błędny news
		echo 'Błędny news!';
	}
}

PS poczytaj sobie jak działa Zend_Registry - przydatny singleton, który możesz wykorzystać w metodzie do zapisu, aby nie bawić się w przekazywanie wszędzie instancji bazy danych:

public function doSave() {
	$db = Registry::get('database');
	
	/* ... */
	
	return true;
}
0

Dlaczego otrzymuję takie błędy?

Notice: Undefined index: title in C:\xampp\htdocs\CMS\add_news.php on line 45

Notice: Undefined index: content in C:\xampp\htdocs\CMS\add_news.php on line 46

Fatal error: Call to undefined method News::setAuthor() in C:\xampp\htdocs\CMS\add_news.php on line 47

Plik z klasą:
http://pastebin.com/yhztSH9U
Kod HTML:
http://pastebin.com/5Cgw7CBQ

Klasa z bazą danych:
http://pastebin.com/hsJCiH1p

Co tu jest nie tak? Jak w takim przypadku się połączyć z bazą danych?

0

Masz napisane co jest nie tak i w której linijce. Komunikaty o błędach zawierają informacje co jest nie tak. Wystarczy je przeczytać i zobaczyć co jest nie tak, a następnie pomyśleć jak można to naprawić. Czasem można też wyszukać błąd w google i wyszukać rozwiązania

0

Tylko, że nie do końca wiem jak to naprawić.

0

No przecież nie masz zadeklarowanych pól topic oraz news...
Ja Ci dałem tylko poglądowy kod - przepisz go na swoje potrzeby, a nie wklejaj na pałę...

0

Ok. W porządku. Zaczynam powoli to ogarniać. Mam jedno pytanie co do takiej metody:

public function isValid(){
	$this->doFilter();
	if(empty($this->subject)){
		print('Pole z tematem jest puste!');
		return false;
	}
	if(empty($this->content)){
		print('Pole z newsem jest puste!');
		return false;
	}
	return true;
}

Tak sobie dodałem printy. Wiem, że metoda nie może ponoć nic printować tylko zwracać dane. Dlaczego zatem nie można tak zrobić? Co jest w tym nie tak? Którą zasadę programowania obiektowego znowu złamię, gdy tak postąpię :) ? Jaki jest sposób na wyświetlanie owych printów?

0

Wywołanie metody gdzieś w kodzie i sprawdzenie zwracanej wartości na zewnątrz. Wtedy sobie możesz wyświetlać co Ci się podoba.

if($obiekt->isValid()){
   echo 'dziala!';
}else{
   echo 'pole z tematem lub newsem jest puste';
}
0

Dlaczego zatem nie można tak zrobić?

Ponieważ nazwa metody wskazuje, że ona tylko coś sprawdza i zwraca true/false (per prefiks is).
Poza tym to isValid niekoniecznie musi być wywołane w momencie, gdzie chciałbyś, aby coś było wyświetlone (zwłaszcza, gdy zaczniesz korzystać z systemu szablonów) lub być może chciałbyś jakoś ładnie opakować ten komunikat błędu (w jakieś <h1>, <div class="alert"> or sth).

Ładniej będzie tak:

class News {
	
	private $errorMessage;
	
	/* ... */
	
	public function isValid() {
		$this->errorMessage = '';
		$this->doFilter();
		
		if (empty($this->subject)) {
			$this->errorMessage = 'Pole z nazwą tematu nie może być puste!';
			return false;
		}
		
		if (empty($this->subject)) {
			$this->errorMessage = 'Pole z treścią newsa nie może być puste!';
			return false;
		}
		
		return true;
	}
	
	/* ... */
	
	/**
	 * Zwraca komunikat błędu.
	 * @return string
	 */
	public function getErrorMessage() {
		return $this->errorMessage;
	}
	
	/* ... */
	
}

Wtedy możesz zmienić także metodę doSave, aby nie rzucała wyjątku, a także zwracała komunikat błędu poprzez getErrorMessage() (aby trzymać się jednej konwencji) i uzyskasz taki ładny kod:

-- kod HTML i PHP --

if ($news->isValid()) {
	if ($news->doSave()) {
		echo 'Zapisano ok!';
	} else {
		echo sprintf('<div class="alert">Wystąpił błąd podczas zapisywania newsa: %s</div>', $news->getErrorMessage());
	}
} else {
	echo sprintf('<div class="alert">Błąd: %s</div>', $news->getErrorMessage());
}

Plus jak tak myślę, mógłbyś zmienić nazwę z isValid na doValidate, będzie lepiej komponowało się z doSave.

1 użytkowników online, w tym zalogowanych: 0, gości: 1