a little hand please (proof read work cant see error)

[Lone_Ranger]

I cannot get my IN MY MIND comment box working, I think it maybe to do with my } { tags that will not forward the page to action=comments.

 

Could someone please check it over and see if they can spot the problem as I have stared and fiddled with it for ages and no luck

controlpanel1.php

[mac_gyver]

you need to tell us what sort of symptoms or errors you are getting to narrow down the problem, since the problem could be something to do with your server/browser, which we cannot duplicate.

[Lone_Ranger]

it does nothing, nothing enters into the database and when you click the submit button instead of going to ?action=comments it goes to ?comments=&submit.x=52&submit,y=27

 

which returns you to the content that is presented on your controlpanel1.php

[mac_gyver]

sorry for the blunt commentary, but the posted code is so far from what it needs to be in order to work. you need to slow down and organize what you trying to do before you try to write the code to do it.

 

for that specific symptom, your form does not have a method specified, so the get method is used as the default. when you have a get method form, the form data replaces anything you coded on the end of the url in the action attribute. also, an image as a submit button submits the coordinates where it was clicked. either use the post method or put the action value into a hidden form field.

 

some issues with the code (most of these are consistency issues and constancy counts in programming) -

 

1) you need to pick one library of database of functions to use and stick to that one library. you apparently have an include file making a msyql_ connection. why are you then making a mysqli connection later in the code, that isn't actually used? btw - the mysql_ functions are depreciated and new code should use either the msyqli or PDO database library of functions.

 

2) you are switching back and forth between using die() and trigger_error() statements. pick one and stick with it.

 

3) you are putting or die()/or trigger_error() statements on the end of string assignment statements. why are you doing that? a string assignment, unless it is an empty string or a zero value won't fail and cause the or ... statement to run. (this is a rhetorical question, i already know why you are doing it, you are copy/pasting things without knowing what they mean, but you must know and have a state-able purpose for every statement you put into your code.) for debugging purposes, you would need to put the or die()/or trigger_error() statements on the end of your mysql_query() statements.

 

4) your code on this page is dependent on someone being logged in. you should test for the logged-in state, first, once, not multiple times and not after you have already tried to use the value in the logged-in session variable.

 

5) you are storing the online/offline status in two tables. this is redundant and just results in more code you have to write and debug.

 

6) it's likely several of your database queries are related and should be consolidated into JOIN'ed queries, but until the code is cleaned up, it's hard to tell what the actual logic/goal of the code is.

 

7) the code handles multiple action== "..." values. the common code on the page shouldn't be repeated within each action block. see this link - http://en.wikipedia.org/wiki/Don%27t_repeat_yourself each action block should produce only the unique output it is responsible for and your basic page layout should be separate and at the end of your code. you would just echo the unique output where it is needed in the page layout.

 

you are running separate database queries getting the online/offline status rows separately. why not use one query, ordering the rows by the online/offline status? it also appears for this specific set of queries that earlier in the code you are querying for this same data to get a count. all three queries can be replaced with one query, then just use the result from that one query as needed.

 

9) related to item #4, ALL the action code on the page would need to test if the current user is logged in and if the input values that action code expects actually exists. however, by separating out the logged-in test and putting it up near the start of the page, you can easily insure that the code on the page will only run if the current visitor is logged in.

 

 

it appears you are just piecing together code without a plan. this just results in excessive and repetitive logic, making it harder for you to keep track of what your code is doing or supposed to do, and making it harder for anyone trying to help with the code to even see what goal the code is trying to accomplish.

Edited December 14, 2013 by mac_gyver

[Lone_Ranger]

your right I am slapping stuff together, I'm getting ideas and then coding it in but am not back tracking to make the code cleaner and easier to navigate. purposely this is my own fault for leaving things when its working instead of tidying it up and a lot of feedback there helps me out.

[Lone_Ranger]

I forgot method = post I been looking at that for ages and never noticed. I will re go over my code and tidy it up like you suggested for it to be more efficient