Page 1 of 1

New to web developing.. Am I doing things right?

Posted: Thu Apr 17, 2014 9:03 pm
by SaW1337
Hello,
I'm new to webdev and still trying to understand what to do and what not to do..
Last night I wrote a small script to help myself with keeping all the notes and ideas in one place. I'm using PHP/HTML/JS(jQuery+ajax) only, whole script is written in one file (except for jquery).
Script took me 2 hours to write, for me it looks like I have done everything correctly, but I doubt it. Could I please get some critique and tips on what (if) I'm doing wrong?

Script:

Code: Select all

<?php 
$con=mysqli_connect("-","-","-","-");
if(isset($_POST["data"])){
	$data=explode(":",$_POST["data"]);
	if($data[0]==1 && preg_match('/^[0-9]*$/', $data[1])) 
		if (mysqli_query($con,"DELETE FROM tracker WHERE id=$data[1]")) echo "<span style=\"color:green\">Entry with id ".$data[1]." deleted!";
	die;
}
if(isset($_POST["edit"]) && preg_match('/^[0-9]*$/', $_POST["id"])) {
	if (mysqli_query($con,"UPDATE tracker SET data='".mysqli_real_escape_string($con,$_POST["value"])."' WHERE id=$_POST[edit]")) echo $_POST["value"];
	die;
}
echo "<head>
	<link href=\"http://fonts.googleapis.com/css?family=Prociono\" rel=\"stylesheet\" type=\"text/css\">
	<script src=\"http://ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js\"></script>
	<script src=\"js.js\"></script>
	<style>
		body { background: url(http://i.imgur.com/6RNBEba.jpg) no-repeat black fixed ; color:white; }
	</style>
	<script>
		$(document).ready(function(){
		$('.edit_area').editable('tracker.php', {
			id: 'edit',
			name: 'value',
			type: 'textarea',
			cancel: 'Cancel',
			submit: 'OK',
			event: 'dblclick',
			indicator: '<img src=\"http://www.appelsiini.net/projects/jeditable/img/indicator.gif\">',
			tooltip: 'Click to edit...'
		});
			$(\".ajax\").click(function(e){ 
				var r=confirm('Are you sure you want to delete this record?'); 
				if (r==true)
				{
					$.ajax({ 
						type: 'POST',
						url: 'tracker.php',
						data: { data: $(this).attr('value')},
						success:function(result){ 
							$('#result').html(result);
							$('#saraksts').html('<font color=\"white\">Loading..</color>');
							$.ajax({
								type: 'POST',
								url: 'tracker.php',
								data: { load: 'saraksts'},
								success:function(result){ $('#saraksts').html(result); }
							});								
						}
					});
				}
			});
		});
	</script>
</head>";
echo "<body>";
if($_POST["submit"]) mysqli_query($con,"INSERT INTO tracker (type,data,time,deleted) VALUES (".mysqli_real_escape_string($con,$_POST["type"]).", '".mysqli_real_escape_string($con,$_POST["text"])."',".time().", 0)");
if(!$_POST["load"]) echo "<center>
	<form action=\"tracker.php\" method=\"POST\">
		<select name=\"type\" style=\"color: white; background-color: #1F1F00\">
			<option value=\"1\">Note</option>
			<option value=\"2\">Important</option>
			<option value=\"3\">Idea</option>
		</select><br/>
		<textarea name=\"text\" rows=\"4\" cols=\"50\" style=\"color: white; background-color: #1F1F00\"></textarea><br/>
		<input type=\"submit\" name=\"submit\" value=\"Submit\" style=\"color: white; background-color: #1F1F00\">
	</form>
	<div id='result'></div>
</center>";
$t=mysqli_query($con,"SELECT * FROM tracker WHERE deleted=FALSE ORDER BY time DESC"); if(mysqli_num_rows($t)){
	echo "<div id=\"saraksts\" align='center' style='width:800px; margin: auto'><table style=\"width:100%;border-collapse: collapse; \">";
	$c; $cc;
	while ($t2 = mysqli_fetch_array($t)) {
	if($c) { $c=FALSE; $cc="#191919"; } else { $c=TRUE; $cc="#303030"; }
	if($t2[2]==1) $ccc="#0066FF"; elseif($t2[2]==2) $ccc="#FF0000"; elseif($t2[2]==3) $ccc="#00FF00";
	echo "<tr>
		<td style=\"background-color:".$ccc.";width:0.5%;\"></td>
		<td style=\"background-color:#1F1F00; width:20%; color:white; border-bottom:dashed grey; border-width:1px;\"> 
			<a class='ajax' value='1:".$t2[1]."'>[<span style=\"color:red\">X</span>]</a> ".date("Y/m/d H:i:s",$t2[4])."
		</td>
		<td style=\"color:white; font-family: 'Prociono', serif; border-bottom:dashed grey; border-width:1px; background-color:".$cc.";opacity:0.85;\">
			<div id='".$t2[1]."' class=\"edit_area\">".str_replace("\\", "", $t2[3])."</div>
		</td>
	</tr>";
	}
	echo "</table></div></body>";
}
I wrote it for personal use only.

Re: New to web developing.. Am I doing things right?

Posted: Sat Apr 19, 2014 4:48 pm
by Celauran
Some quick observations, point form.
  • No HTML tags? No doctype declaration? No charset specified?
  • You're echoing everything. Why? Don't do that.
  • As much as possible, avoid inline styles.
  • Escaping your inputs is good. Prepared statements are better.
  • Use braces. Always.
  • Does this really all belong in the same file?

Re: New to web developing.. Am I doing things right?

Posted: Mon Apr 21, 2014 10:50 am
by SaW1337
  • You're echoing everything. Why? Don't do that. I thought it's better than closing/opening <?php ?> all the time..
  • Use braces. Always. I thought you don't need to open a brace for single function.. Increases readability. Could you please explain how is this bad?
  • Does this really all belong in the same file? I usually don't do that but this time script was kinda small and it felt unnecessary to make includes for 5 line codes
Thank you for the tips :)

Re: New to web developing.. Am I doing things right?

Posted: Mon Apr 21, 2014 11:10 am
by Celauran
SaW1337 wrote:You're echoing everything. Why? Don't do that. I thought it's better than closing/opening <?php ?> all the time..
I think it hurts readability. You lose syntax highlighting, making mistakes harder to spot. Much easier to break by forgetting to escape a double quote somewhere. I keep my views and templates bereft of PHP save for flow control.
SaW1337 wrote:Use braces. Always. I thought you don't need to open a brace for single function.. Increases readability. Could you please explain how is this bad?
You don't technically, as evidenced by the fact that your script is working fine. Adding a second line inside the conditional and not noticing the missing braces can potentially lead to a whole bunch of frustration. Those two extra characters don't really cost anything but can save some headaches down the road. Also, I find the braces improve readability, though that's a lot more subjective.

Re: New to web developing.. Am I doing things right?

Posted: Thu Jul 10, 2014 6:55 pm
by SaW1337
Hello again.. Been reading and learning PHP for some time now.
I made login and autologin 2 in 1 (for my upcoming script). I still don't use braces for single function, somehow I don't like it and it improves readability for me.

What makes me wonder is..
1) Is this script safe (I know security is an illusion, but against majority of hackers)?
2) I'm storing logins in MySQLi Engine=MEMORY (ram) so it reads login faster.. is this a right thing to do, or is mysql caching good enough?

Script: http://pastebin.com/eqw42TMd (no html)

Re: New to web developing.. Am I doing things right?

Posted: Thu Jul 10, 2014 7:16 pm
by Celauran

Code: Select all

$password=isset($_POST['password']) ? $_POST['password'] : $_SESSION["password"];
...
mysqli_prepare($con, "SELECT u_id FROM accounts WHERE u_username=? AND u_password=? LIMIT 1")
...
mysqli_stmt_bind_param($sql, "ss", $username, $password);
Looks like you're storing passwords in plain text. If your database is ever compromised, you have potentially hurt your users. People shouldn't use the same password on multiple sites, but they do. Hash your passwords. Use bcrypt.

Code: Select all

if(!ctype_alnum($username.$password)) $err[].="Username and Password must contain only alphanumeric symbols!";
You also don't want to put unnecessary restrictions on passwords. Requiring a digit and a special character? OK. Makes sense. Forbidding special characters reduces the number of possible passwords, making cracking them easier.

Re: New to web developing.. Am I doing things right?

Posted: Thu Jul 10, 2014 10:32 pm
by Christopher
One of the biggest things you should do is separate your Domain code from your Presentation code. I would recommend putting all the code the loads/saves data to the tracker table into a Tracker or TrackerModel class. It should have methods like insert(), update(), delete() and findAvailable() (not sure what deleted=FALSE means!?). Then have your page templates access that object. It will keep all code together for reuse, refactoring, etc.

Re: New to web developing.. Am I doing things right?

Posted: Fri Jul 11, 2014 5:18 am
by SaW1337
Celauran wrote:

Code: Select all

$password=isset($_POST['password']) ? $_POST['password'] : $_SESSION["password"];
...
mysqli_prepare($con, "SELECT u_id FROM accounts WHERE u_username=? AND u_password=? LIMIT 1")
...
mysqli_stmt_bind_param($sql, "ss", $username, $password);
Looks like you're storing passwords in plain text. If your database is ever compromised, you have potentially hurt your users. People shouldn't use the same password on multiple sites, but they do. Hash your passwords. Use bcrypt.

Code: Select all

if(!ctype_alnum($username.$password)) $err[].="Username and Password must contain only alphanumeric symbols!";
You also don't want to put unnecessary restrictions on passwords. Requiring a digit and a special character? OK. Makes sense. Forbidding special characters reduces the number of possible passwords, making cracking them easier.
Thanks for tips, will look into bcrypt and will put much less restrictions on passwords.
Christopher wrote:One of the biggest things you should do is separate your Domain code from your Presentation code. I would recommend putting all the code the loads/saves data to the tracker table into a Tracker or TrackerModel class. It should have methods like insert(), update(), delete() and findAvailable() (not sure what deleted=FALSE means!?). Then have your page templates access that object. It will keep all code together for reuse, refactoring, etc.
I'm now doing PHP first and template only later, as an include (as you can see, there's no HTML in my second code).I don't understand about Tracker/TrackerModel class.. Is that some kind of framework? Also, if that's something to do with OOP - I'm trying to avoid it. I spent hours researching both Procedural and OOP, came to an conclusion that 99.9% of the time OOP is not necessary and it only slows down compiling times. I don't think I will ever need those special features OOP offers.. OOP vs Pro. syntax is purely subjective and I like Pro. better, but I can understand why some people prefer OOP (readability, organized, etc.)

Re: New to web developing.. Am I doing things right?

Posted: Fri Jul 11, 2014 1:13 pm
by Christopher
SaW1337 wrote:Also, if that's something to do with OOP - I'm trying to avoid it. I spent hours researching both Procedural and OOP, came to an conclusion that 99.9% of the time OOP is not necessary
You are incorrect in your conclusion and are simply limiting yourself to a subset of the language syntax to your detriment. But you will prove that to yourself the hard way in the long run (trust me).
SaW1337 wrote: and it only slows down compiling times.
PHP is not compiled and you will not notice any performance difference on any site you will build.
SaW1337 wrote:I don't think I will ever need those special features OOP offers.. OOP vs Pro. syntax is purely subjective and I like Pro. better, but I can understand why some people prefer OOP (readability, organized, etc.)
They are not special features and it is not purely subjective. Conversely almost all the problems with the design of your code are due to the lack of use of OOP. And yes, people always argue that anything you can do in OOP you can do procedurally. But there are no major PHP frameworks or applications that are procedural only. It is just not done because it is so inferior.

Re: New to web developing.. Am I doing things right?

Posted: Sat Jul 12, 2014 8:34 am
by SaW1337
Could you please explain further and maybe give some example or some article? If you're correct, my findings about OOP were horrible and maybe I really should learn it.

Re: New to web developing.. Am I doing things right?

Posted: Sat Jul 12, 2014 12:42 pm
by Christopher
You don't need to go OOP crazy or be an OOP zealot (please don't), but limiting yourself to only part of a language's syntax -- especially a very useful part -- is unwise. At its simplest, the class construct is a nice way to namespace functions. Objects are essentially data structures with a namespace of functions that go with them. It is very useful to pass around data structures instead of having lots of individual variables littering your code. And since the advent of OO, there are a bunch of useful design concepts (and may reusable good design practices called patterns) that OO enables. These simply expand the available tools you can use to solve software problems. There's neither magic nor politics behind OO -- they are just practical part programming.