0
\$\begingroup\$

I was able to run the form to update the data of the user profile, for the moment is in a rough version, but I know that can be improved.

I post what I did, I hope you could help me to improve the code that definitely is unclear and with defects.

//form update
if ((isset($_POST['submit'])) || (isset($_POST['email'], $_POST['location'], $_POST['gender'] ))) {

    $email = $_POST['email'];
    $location = $_POST['location'];
    $gender = $_POST['gender'];

    $stmt = $db->prepare("UPDATE members SET email=?, location=?, gender=? WHERE memberID = 1");
    $stmt->execute(array($email, $loc, $gender));
    $affected_rows = $stmt->rowCount();
}

And,

<form role="form" method="POST" action="">
    <div class="form-group">
    <label class="control-label">Email</label>
    <input type="text" name="email" class="form-control" value="<?php echo $_SESSION['email'] ?>" />
    </div>

    <div class="form-group">        
    <label class="control-label">Location</label>   
    <input type="text" name="location" class="form-control" value="<?php echo $_SESSION['location'] ?>" />       
    </div>

    <div class="form-group">
    <label class="control-label">Gender</label>
    <input type="text" name="gender" class="form-control" value="<?php echo $_SESSION['gender'] ?>" />
    </div>

<div class="margiv-top-10">
<input type="submit" name="submit" class="btn" value="Update" >
<a href="account.php" class="btn btn-default">Back</a>
</div>
</form>
\$\endgroup\$
2
\$\begingroup\$

Front-end

I would remove the form's action="" attribute and any of the closing slashes in of your input tags, assuming you're using HTML5.

Instead of <?php echo $_SESSION['email'] ?>, you could do <?= $_SESSION['email']; ?>, etc.

I'd advise you to make a habit of always ending with a semicolon, even when it's not necessary.

Back-end

Here's what I would do:

  • Remove the comment, since it doesn't provide any value.
  • Remove the intermediate variables, since there is no need for them.
  • Remove storing the rowCount() (unless you're using it somewhere).
  • Rewrite the if-expression.

Regarding the following expression:

(isset($_POST['submit'])) || (isset($_POST['email'], $_POST['location'], $_POST['gender']))

Evaluating to true if $_POST['submit'] is set or $_POST['email'], $_POST['location'], $_POST['gender'] are all set. Not very safe, assuming you require every field to be filled based on looking at how your query is built.

I would just end up with the following code and if you wanted to do some specific things in a scenario where one of the fields has not been filled you can just add an else to the if statement.

if (isset($_POST['email'], $_POST['location'], $_POST['gender'])) {
    $stmt = $db->prepare("UPDATE members SET email = ?, location = ?, gender = ? WHERE memberID = 1");
    $stmt->execute([$_POST['email'], $_POST['location'], $_POST['gender']]);
}

You might find my answer a bit sloppy since I haven't answered any questions for a long time, but hopefully you find it to be helpful.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your reply. The update works fine, only that in the input value is not shown to date. For example, if I update the field "Location", is updated correctly in the database but is not shown in the input field: <input type="text" name="location" class="form-control" value="<?= $_SESSION['location']; ?>" /> How can I do? Perhaps it would be appropriate to create a class that taps the user data directly from the database? \$\endgroup\$ – Marco Apr 15 '15 at 8:31
  • \$\begingroup\$ @Marco Really, you should head over to StackOverflow for an answer on this question. CodeReview only handles working code. But, with the same way you populated the session with the user data beforehand should you fix this issue. \$\endgroup\$ – Kid Diamond Apr 15 '15 at 8:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.