CODEWITHSUNDEEP
phplogic
MEM
MEM

Reputation: 31337

Logic Programming Help - II

A 

if (  ( empty($infoA) || empty($infoB) ) && ( !empty($inputA) && !empty($inputB) )  ) 
{
  //add
}

B

if (  ( !empty($infoA) || !empty($infoB) ) && ( empty($inputA) && empty($inputB) )  )
{
  //remove
}

C

if (  ( !empty($infoA) || !empty($infoB) ) && ( ($inputA != $infoA) || ($inputB != $infoB) )  ) 
{
  //add
}

So, in order to not repeat the add, we can:

if (A || C) 
{
  //add
}
elseif(B) 
{
 //remove
}

Any better logic to be applied here on your option?

Context: I believe it's irrelevant what this should do. I mean, it's a logical question. :s Not sure what to write here... :(

This is for a form: some inputs will come from the database, others from input fields. We are doing some comparisons here.

A context: If the value that comes from the database is empty, and the input fields A and B are NOT empty, do add to database.

B context: If the value that comes from the database is NOT empty, and the input fields A and B are empty, do remove from the database.

C context: If the values that comes from the database are NOT empty, AND the input fieldA !equal infoA or input fieldB NOT equal database value infoB then, do add to the database.

Please advice. MEM

Upvotes: 2

Views: 666

Answers (2)

Simon Gillbee
Simon Gillbee

Reputation: 3982

Several questions:

  • Is C really Add, or is it update?
  • Do you really need to tie procesing of inputA and inputB with infoA and infoB?

In general, I would refactor this by:

  • Splitting the add/remove decision into a separate process_input function which handled just one pair of input/info objects.
  • Spliting add/remove activities into their own functions which would handle basic empty handling of the data.

The only simplification of the logic is that the decision for calling add() or remove() is only based on whether or not the input data is empty or not. That's all that's really needed. The add() and remove() functions do their own data validation to make sure the operation is valid for the supplied data.

This refactoring demonstrates two important programming techniques:

  • The DRY (don't-repeat-yourself) principle... we shouldn't be checking empty() on an object more than once.
  • SRP (single-responsibility-principle) - each operation should only do one thing.
    • The input handler is responsible for pairing the input data with the existing info data and then calling process_input.
    • The process_input function is only responsible for deciding whether we need to do an add() or remove() operation.
    • The add() and remove() functions are each responsible for their own distinct operation which can be tested in isolation of the rest of the code

(Excuse my php... it's a bit rusty)

function remove($data)
{
    if (empty($data))
        return;

    // Do remove operation
}

function add($data)
{
    if (empty($data))
        return;

    // Do add operation
}

function process_input($input, $info)
{
    if (empty($input))
        remove($info);
    else
        add($input);
}

process_input($inputA, $infoA);
process_input($inputB, $infoB);

Upvotes: 1

Extrakun
Extrakun

Reputation: 19305

I think unrolling the conditions and using else or else-if statements helps a lot, for readability and understanding of code. While your proposed answer cuts down on lines of code, it is going to be hard to understand and debug, especially if you pass it on to someone else.

Here's my take

// check if to update in DB or not

$value = db_adapter::get_var($sql);
if (empty($value))
{
  if (!empty($field_A) && !empty($field_B))
    add($field_A, $field_B);  

} else 
// you can roll the following block up into a function
{


  if (empty($field_A) && empty($field_B))
    remove($field_A, $field_B);
   else {
      if ($field_A != $value)
       update($field_A);  
     else if ($field_B != $value)
       update($field_B);
   }

}

Each of the else can be rolled up into a function; I didn't because I can't think of anything logical to call the block of code in else block (process-maybe-delete-update? determine-update-or-delete?)

Upvotes: 1

Related Questions