Noskiw Posted January 27, 2014 Share Posted January 27, 2014 So I'm trying to teach myself how to construct more robust code, and I thought using Object Oriented Programming would help with that. When I try and echo out $_SESSION['user'] it is claiming that there is an undefined index. Now I now that I could use if(isset($_SESSION['user'])) but you'll see in a second why I think there's a bit of a problem This is my class code <?php class blogClass { function __construct() { $loggedInCheck = $this->CheckIfUserLoggedIn(); $this->echoUserData($loggedInCheck); } private function CheckIfUserLoggedIn() { if(isset($_SESSION['user'])) return false; else if(isset($_COOKIE['userCookie'])) return true; else return null; } private function echoUserData($log) { if(!$log) echo $_SESSION['user']; else if($log) echo $_COOKIE['userCookie']; else if($log == null) echo "NOT LOGGED IN"; } } ?> And this is the index page <?php session_start(); include '.././inc/PHP/classes/blogClass.php'; $blogClassObject = new blogClass; ?> So I really don't know what the problem is here. Any help would be appreciated, thanks. Quote Link to comment Share on other sites More sharing options...
gizmola Posted January 27, 2014 Share Posted January 27, 2014 You're trying to utilize null and isset() in a very odd manner. The immediate problem is that when you try (if (!$log) and you pass null to that, the condition is going to be true. With that said, this really isn't good coding philosophy: function CheckIfUserLoggedIn() { if(isset($_SESSION['user'])) return false; This appears to do the opposite of what the naming suggests. Why would a logged in user (due to the presence of a session variable) return false? Try and have your naming match the intention of the code, and break things down into discreet pieces that do one thing. Quote Link to comment Share on other sites More sharing options...
Noskiw Posted January 27, 2014 Author Share Posted January 27, 2014 I see why you're confused. I'm returning false if it is stored as a session and returning true if stored as a cookie, and then null if not stored at all. It's simply because of the acceptance or denial of the usage of cookies on the website. Quote Link to comment Share on other sites More sharing options...
ginerjm Posted January 27, 2014 Share Posted January 27, 2014 It's simply a poorly designed piece of code. You've got 20 lines of code that people are looking at and advising you to change simply because it's not readable. For something that is simply simple, you've made it difficult to understand. Why use a Boolean value, which is intended to have TWO values when you have a condition that could have THREE answers? Why make it so complicated when you could simply return SESSION,COOKIE or NONE as your answer? Even you can't figure out this logic to determine what is wrong with the code. As for your current problem, you are asking it to display a variable ($_SESSION['user']) that doesn't exist. Simple as that. Your own code has determined that to be true, but your use of that code is making you try and display it anyway. Try to write code that is readable and understandable by another person completely unfamiliar with the workings of it. If you are ever going to write "robust code" for a bigger project someday, where others may have to work on it after you, you will have to ensure that it can be understood. Good luck! Quote Link to comment Share on other sites More sharing options...
gizmola Posted January 27, 2014 Share Posted January 27, 2014 I see why you're confused. I'm returning false if it is stored as a session and returning true if stored as a cookie, and then null if not stored at all. It's simply because of the acceptance or denial of the usage of cookies on the website. I understand what your code does, and pinpointed why you are getting a result that surprises you. It is because you never get to the condition that checks for the variable == null. Thus you attempt to echo out a variable that is not set. Thus you get a notice. The rest of the advice provided and reitereated by ginerjm has to do with helping you see how you can write better clearer more maintainable code, whiich ostensibly is the entire reason you're trying to use OOP in the first place, right? Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.