I have a very concrete scenario but I think it would be quite frequent.

I’m trying to come up with a solution that it’s “better” by better meaning more scalable, reusable and not smelly.

I’m working with very old crufty legacy code and Trying to improve it one thing at a time.
Right now I have a process in which users can “vote” “things”.

The process to determine if a user can vote is a complex logic and it may be subject to change.

Right now I have two functions, one (call it can_vote() ) takes the user and the thing and returns true or false. It’s used to know what to display to the user.

The other one is vote() It takes user and thing and and checks again all the logic that can_vote checks but instead of returning false on error throws an error that gets displayed to the user.

The system tries to never get to a state in which the user would be able to use “vote” if the result would be an exception but it still happens.

I know there may be a thousand flaws to the whole system but right now the only thing I’m trying to do is unite the logic of the two functions in one single place in the better way (as defined above).

Ideally I would like to be able to call can_vote in my controllers just getting a true or false and acting acordingly.
can_vote, being a binary question should return true|false.
I could be able to use the logic inside can_vote in places where I want to display information about “false” responses.

I’ve got multiple ideas about how to solve that but all of them seem… weird:

Possible solutions

a) use can_vote(user,thing,verbose)

verbose beeing true false
if true returns string with the error
if false returns true|false

public can_vote ( user, thing, verbose ) {
    if( user is not logged ) {
        if( verbose ) {
            throw user is not logged
        }
        return false
    }

    //all other checks
    return true
}

cons

• returns different types depending on parameters
• you get an if else that you carry arround the function

b) use a private function to determine if can vote. Use two helper functions

that would be something like that:

private can_really_vote ( user, thing ) {
    if( user is not logged ) {
        throw user is not logged
    }

    //all other checks
    return true

}

public can_vote ( user, thing ) {
    try {
        can_vote = this->can_really_vote( user, thing )
    }
    catch {
        //can_really_vote threw an exception
        return false
    }

    return false
}

public vote ( user, thing ) {
    if ( this->can_vote( user, thing ) ) {
        //do the vote thingy
    }
    else {
        // you shouldn't even get here. The user should have gotten an exception from can_really_vote
    }
}

c) what php usually does

That would be something like

public vote () {
    if ( can_vote() ) {
        //carry on with the voting
    }
    else {

        display can_vote_last_error()
    }
}

Things like that are usually seen in database access classes and the like.
Really? Keeping an internal trace of all those silly errors like “you are not logged”, “the contest is over” and all that stuff just to be able to recall them?
It seems quite complicated.

Of course It can be done better, for example with dedicated exceptions… doesn’t seem right dough.

I’m sure I can get more solutions… the problem boils down to wanting to get a true/false result in one case and some text on another for the same logic.

I guess the problem is quite common so there should be some software pattern for this.
Or maybe I’m being unable to abstract from it so I see the obvious elegant solution.

What would you do?

Thanks!

PS I’m almost sure it doesn’t matter but I’m working in vanilla PHP 5.3