Activation Controller if statements

[eluser]xtremer360[/eluser]
Curious to know what everyone thinks of my controller and what if anythign I could do to eliminate so many nested ifs.

http://pastebin.com/M2PpyMy7

[eluser]DarkManX[/eluser]
you have some unecessary if's like:

Code:

if ($this->users_model->change_account_type($user_id, 2))

in which case the stuff would return not true?

anyway you should some all the stuff up in a model method and just run like $this->users_model->activate($user_id) in the controller to maintain clean code. many of your model methods just updating a certain column in a row without format the data somehow. you can make a magic method for achieve that, you dont have to create a method for each column.

but still: if you want to maintain such logic, you should but the messages on the top, the success logic to the top, like:

Code:

if(//something){
$err[] = 'Message';
}elseif(//do something){
$err[] = 'Other Message';
}
// other if's
if(count($err)>0){
// message output
}else{
// success logic
}

that how i used to code when i didnt used a framework!

[eluser]xtremer360[/eluser]
[quote author="DarkManX" date="1346227785"]you have some unecessary if's like:

Code:

if ($this->users_model->change_account_type($user_id, 2))

in which case the stuff would return not true?
[/quote]

Well how would I handle it if it didn't update then. The user should be told that something went wrong and their status wasn't changed. Because otherwise your going to leave them clueless and not know what happened.

[quote author="DarkManX" date="1346227785"]
anyway you should some all the stuff up in a model method and just run like $this->users_model->activate($user_id) in the controller to maintain clean code. many of your model methods just updating a certain column in a row without format the data somehow. you can make a magic method for achieve that, you dont have to create a method for each column.
[/quote]

I don't like doing that because I'm going to only do this once and that's in the register controller so why make a special function that I'm only going to use once. Wouldn't make sense.

[eluser]DarkManX[/eluser]
1. well, if you script isnt able to put something into the db its not a common failure due to user input. it wont insert/update something, if no db connection or the table is not how it supossed to be. you cant just check all interactions with your db. u gotta assume that your db settings work. otherwise you sould work with exceptions.

2. what about your admin interface? if you want to activate a user manually you will have to copy&paste; the whole controller logic once more... otherwise you would have to copy only 1 line of code. well, you might think, ok i will just going to have this logic twice, np for me to copy it 1 time. so what about changes on the activation script? you will have to edit the both scripts, no good!

beside that just to it the good way regardless you need it or not. it doesnt take up too much extra time and will make your code much nicer and editable. as i told in some previous thread, you gotta maintain consistency. dont code one way in a controller and the other way in another.