Code cleanup.

app/controllers/PiggybankController.php

@@ -290,11 +290,12 @@ class PiggyBankController extends BaseController
         Session::flash('errors', $messages['errors']);
         if ($messages['errors']->count() > 0) {
             Session::flash('error', 'Could not store piggy bank: ' . $messages['errors']->first());
+            return Redirect::route('piggy_banks.create')->withInput();
         }
 
 
         // return to create screen:
-        if ($data['post_submit_action'] == 'validate_only' || $messages['errors']->count() > 0) {
+        if ($data['post_submit_action'] == 'validate_only') {
             return Redirect::route('piggy_banks.create')->withInput();
         }
 

app/controllers/RepeatedExpenseController.php

@@ -178,6 +178,8 @@ class RepeatedExpenseController extends BaseController
     }
 
     /**
+     * @SuppressWarnings("CyclomaticComplexity") // It's exactly 5. So I don't mind.
+     *
      * @param PiggyBank $repeatedExpense
      *
      * @return $this
@@ -194,7 +196,6 @@ class RepeatedExpenseController extends BaseController
         $data['remind_me']     = isset($data['remind_me']) ? 1 : 0;
         $data['user_id']       = Auth::user()->id;
 
-        // always validate:
         $messages = $this->_repository->validate($data);
 
         Session::flash('warnings', $messages['warnings']);
@@ -202,10 +203,11 @@ class RepeatedExpenseController extends BaseController
         Session::flash('errors', $messages['errors']);
         if ($messages['errors']->count() > 0) {
             Session::flash('error', 'Could not update repeated expense: ' . $messages['errors']->first());
+            return Redirect::route('repeated.edit', $repeatedExpense->id)->withInput();
         }
 
         // return to update screen:
-        if ($data['post_submit_action'] == 'validate_only' || $messages['errors']->count() > 0) {
+        if ($data['post_submit_action'] == 'validate_only') {
             return Redirect::route('repeated.edit', $repeatedExpense->id)->withInput();
         }
 

app/controllers/ReportController.php

@@ -48,15 +48,15 @@ class ReportController extends BaseController
         } catch (Exception $e) {
             return View::make('error')->with('message', 'Invalid date');
         }
-        $date     = new Carbon($year . '-' . $month . '-01');
+        $date         = new Carbon($year . '-' . $month . '-01');
-        $dayEarly = clone $date;
+        $dayEarly     = clone $date;
         $subTitle     = 'Budget report for ' . $date->format('F Y');
         $subTitleIcon = 'fa-calendar';
-        $dayEarly = $dayEarly->subDay();
+        $dayEarly     = $dayEarly->subDay();
-        $accounts = $this->_repository->getAccountListBudgetOverview($date);
+        $accounts     = $this->_repository->getAccountListBudgetOverview($date);
-        $budgets  = $this->_repository->getBudgetsForMonth($date);
+        $budgets      = $this->_repository->getBudgetsForMonth($date);
 
-        return View::make('reports.budget', compact('subTitle','subTitleIcon','date', 'accounts', 'budgets', 'dayEarly'));
+        return View::make('reports.budget', compact('subTitle', 'subTitleIcon', 'date', 'accounts', 'budgets', 'dayEarly'));
 
     }
 

app/controllers/TransactionController.php

@@ -230,6 +230,8 @@ class TransactionController extends BaseController
     }
 
     /**
+     * @SuppressWarnings("CyclomaticComplexity") // It's exactly 5. So I don't mind.
+     *
      * @param $what
      *
      * @return $this|\Illuminate\Http\RedirectResponse
@@ -245,8 +247,6 @@ class TransactionController extends BaseController
         $data['completed']               = 0;
         $data['what']                    = $what;
         $data['currency']                = 'EUR';
-
-        // always validate:
         $messages = $this->_repository->validate($data);
 
         Session::flash('warnings', $messages['warnings']);
@@ -257,17 +257,13 @@ class TransactionController extends BaseController
             return Redirect::route('transactions.create', $data['what'])->withInput();
         }
 
-        // return to create screen:
         if ($data['post_submit_action'] == 'validate_only') {
             return Redirect::route('transactions.create', $data['what'])->withInput();
         }
 
-        // store
         $journal = $this->_repository->store($data);
         Event::fire('transactionJournal.store', [$journal, Input::get('piggy_bank_id')]); // new and used.
-        /*
+
-         * Also trigger on both transactions.
-         */
         /** @var Transaction $transaction */
         foreach ($journal->transactions as $transaction) {
             Event::fire('transaction.store', [$transaction]);
@@ -303,8 +299,9 @@ class TransactionController extends BaseController
         Session::flash('errors', $messages['errors']);
         if ($messages['errors']->count() > 0) {
             Session::flash('error', 'Could not update transaction: ' . $messages['errors']->first());
+            return Redirect::route('transactions.edit', $journal->id)->withInput();
         }
-        if ($data['post_submit_action'] == 'validate_only' || $messages['errors']->count() > 0) {
+        if ($data['post_submit_action'] == 'validate_only') {
             return Redirect::route('transactions.edit', $journal->id)->withInput();
         }
         $this->_repository->update($journal, $data);

app/database/seeds/TestDataSeeder.php

@@ -184,6 +184,9 @@ class TestDataSeeder extends Seeder
     }
 
     /**
+     * @SuppressWarnings(PHPMD.ShortVariable)
+     * @SuppressWarnings(PHPMD.ExcessiveParameterList)
+     *
      * @param Account              $from
      * @param Account              $to
      * @param                      $amount

app/lib/FireflyIII/Database/Account/Account.php

@@ -150,6 +150,7 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte
     }
 
     /**
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength) // cannot make it shorter because of query.
      * @param Eloquent $model
      *
      * @return bool
@@ -159,25 +160,25 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte
         $journals     = \TransactionJournal::whereIn(
             'id', function (QueryBuilder $query) use ($model) {
             $query->select('transaction_journal_id')
-                  ->from('transactions')->whereIn(
+                  ->from('transactions')
-                    'account_id', function (QueryBuilder $query) use ($model) {
+                  ->whereIn(
-                    $query
+                      'account_id', function (QueryBuilder $query) use ($model) {
-                        ->select('id')
+                      $query
-                        ->from('accounts')
+                          ->select('id')->from('accounts')
-                        ->where(
+                          ->where(
-                            function (QueryBuilder $q) use ($model) {
+                              function (QueryBuilder $q) use ($model) {
-                                $q->where('id', $model->id);
+                                  $q->where('id', $model->id);
-                                $q->orWhere(
+                                  $q->orWhere(
-                                    function (QueryBuilder $q) use ($model) {
+                                      function (QueryBuilder $q) use ($model) {
-                                        $q->where('accounts.name', 'LIKE', '%' . $model->name . '%');
+                                          $q->where('accounts.name', 'LIKE', '%' . $model->name . '%');
-                                        $q->where('accounts.account_type_id', 3);
+                                          $q->where('accounts.account_type_id', 3);
-                                        $q->where('accounts.active', 0);
+                                          $q->where('accounts.active', 0);
-                                    }
+                                      }
-                                );
+                                  );
-                            }
+                              }
-                        )->where('accounts.user_id', $this->getUser()->id);
+                          )->where('accounts.user_id', $this->getUser()->id);
-                }
+                  }
-                )->get();
+                  )->get();
         }
         )->get();
         $transactions = [];
@@ -218,9 +219,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte
     public function store(array $data)
     {
 
-        /*
-         * Find account type.
-         */
         /** @var \FireflyIII\Database\AccountType\AccountType $acctType */
         $acctType = \App::make('FireflyIII\Database\AccountType\AccountType');
 
@@ -230,7 +228,6 @@ class Account implements CUDInterface, CommonDatabaseCallsInterface, AccountInte
         $data['account_type_id'] = $accountType->id;
         $data['active']          = isset($data['active']) && $data['active'] === '1' ? 1 : 0;
 
-
         $data    = array_except($data, ['_token', 'what']);
         $account = new \Account($data);
         if (!$account->isValid()) {

app/lib/FireflyIII/Database/TransactionJournal/TransactionJournal.php

@@ -363,11 +363,13 @@ class TransactionJournal implements TransactionJournalInterface, CUDInterface, C
                     $errors->add('account_id', 'Invalid account.');
                 }
                 break;
+            // often seen in deposits
             case (isset($model['account_id']) && isset($model['revenue_account'])):
                 if (intval($model['account_id']) < 1) {
                     $errors->add('account_id', 'Invalid account.');
                 }
                 break;
+            // often seen in transfers
             case (isset($model['account_from_id']) && isset($model['account_to_id'])):
                 if (intval($model['account_from_id']) < 1 || intval($model['account_from_id']) < 1) {
                     $errors->add('account_from_id', 'Invalid account selected.');

app/lib/FireflyIII/Event/Piggybank.php

@@ -180,6 +180,7 @@ class PiggyBank
     }
 
     /**
+     *
      * Validates the presence of repetitions for all repeated expenses!
      */
     public function validateRepeatedExpenses()
@@ -189,24 +190,20 @@ class PiggyBank
         }
         /** @var \FireflyIII\Database\PiggyBank\RepeatedExpense $repository */
         $repository = \App::make('FireflyIII\Database\PiggyBank\RepeatedExpense');
-
         $list  = $repository->get();
         $today = Carbon::now();
-
         /** @var \PiggyBank $entry */
         foreach ($list as $entry) {
-            $start  = $entry->startdate;
+            $count = $entry->piggyBankrepetitions()->starts($entry->startdate)->targets($entry->targetdate)->count();
-            $target = $entry->targetdate;
-            $count = $entry->piggyBankrepetitions()->starts($start)->targets($target)->count();
             if ($count == 0) {
                 $repetition = new \PiggyBankRepetition;
                 $repetition->piggyBank()->associate($entry);
-                $repetition->startdate     = $start;
+                $repetition->startdate     = $entry->startdate;
-                $repetition->targetdate    = $target;
+                $repetition->targetdate    = $entry->targetdate;
                 $repetition->currentamount = 0;
                 $repetition->save();
             }
-            $currentTarget = clone $target;
+            $currentTarget = clone $entry->startdate;
             $currentStart  = null;
             while ($currentTarget < $today) {
                 $currentStart  = \DateKit::subtractPeriod($currentTarget, $entry->rep_length, 0);

app/lib/FireflyIII/Report/Report.php

@@ -232,6 +232,8 @@ class Report implements ReportInterface
      *
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      *
+     * TODO: This method runs two queries which are only marginally different. Try and combine these.
+     *
      * @param Carbon $date
      * @param bool   $shared
      *
@@ -245,7 +247,6 @@ class Report implements ReportInterface
         $end->endOfMonth();
         $userId = $this->_accounts->getUser()->id;
 
-
         $list = \TransactionJournal::leftJoin('transactions', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
                                    ->leftJoin('accounts', 'transactions.account_id', '=', 'accounts.id')
                                    ->leftJoin(

app/lib/FireflyIII/Report/ReportQuery.php

@@ -367,8 +367,6 @@ class ReportQuery implements ReportQueryInterface
                                           );
                                       }
                                   )
-            //                                  ->where('transaction_types.type', 'Deposit')
-            //                                  ->where('acm_to.data', '!=', '"sharedExpense"')
                                   ->before($end)->after($start)
                                   ->where('transaction_journals.user_id', \Auth::user()->id)
                                   ->groupBy('t_from.account_id')->orderBy('amount')

app/lib/FireflyIII/Shared/Toolkit/Date.php

@@ -183,39 +183,34 @@ class Date
     public function startOfPeriod(Carbon $theDate, $repeatFreq)
     {
         $date = clone $theDate;
-        switch ($repeatFreq) {
-            default:
-                throw new FireflyException('Cannot do startOfPeriod for $repeat_freq ' . $repeatFreq);
-                break;
-            case 'daily':
-                $date->startOfDay();
-                break;
-            case 'week':
-            case 'weekly':
-                $date->startOfWeek();
-                break;
-            case 'month':
-            case 'monthly':
-                $date->startOfMonth();
-                break;
-            case 'quarter':
-            case 'quarterly':
-                $date->firstOfQuarter();
-                break;
-            case 'half-year':
-                $month = intval($date->format('m'));
-                $date->startOfYear();
-                if ($month >= 7) {
-                    $date->addMonths(6);
-                }
-                break;
-            case 'year':
-            case 'yearly':
-                $date->startOfYear();
-                break;
-        }
 
-        return $date;
+        $functionMap = [
+            'daily'   => 'startOfDay',
+            'week'    => 'startOfWeek',
+            'weekly'  => 'startOfWeek',
+            'month'   => 'startOfMonth',
+            'monthly' => 'startOfMonth',
+            'quarter' => 'firstOfQuarter',
+            'quartly' => 'firstOfQuarter',
+            'year'    => 'startOfYear',
+            'yearly'  => 'startOfYear',
+        ];
+        if (isset($functionMap[$repeatFreq])) {
+            $function = $functionMap[$repeatFreq];
+            $date->$function();
+
+            return $date;
+        }
+        if ($repeatFreq == 'half-year') {
+            $month = intval($date->format('m'));
+            $date->startOfYear();
+            if ($month >= 7) {
+                $date->addMonths(6);
+            }
+
+            return $date;
+        }
+        throw new FireflyException('Cannot do startOfPeriod for $repeat_freq ' . $repeatFreq);
     }
 
     /**
@@ -229,38 +224,35 @@ class Date
     public function subtractPeriod(Carbon $theDate, $repeatFreq, $subtract = 1)
     {
         $date = clone $theDate;
-        switch ($repeatFreq) {
+
-            default:
+        $functionMap = [
-                throw new FireflyException('Cannot do subtractPeriod for $repeat_freq ' . $repeatFreq);
+            'daily'   => 'subDays',
-                break;
+            'week'    => 'subWeeks',
-            case 'day':
+            'weekly'  => 'subWeeks',
-            case 'daily':
+            'month'   => 'subMonths',
-                $date->subDays($subtract);
+            'monthly' => 'subMonths',
-                break;
+            'year'    => 'subYears',
-            case 'week':
+            'yearly'  => 'subYears',
-            case 'weekly':
+        ];
-                $date->subWeeks($subtract);
+        $modifierMap = [
-                break;
+            'quarter'   => 3,
-            case 'month':
+            'quarterly' => 3,
-            case 'monthly':
+            'half-year' => 6,
-                $date->subMonths($subtract);
+        ];
-                break;
+        if (isset($functionMap[$repeatFreq])) {
-            case 'quarter':
+            $function = $functionMap[$repeatFreq];
-            case 'quarterly':
+            $date->$function($subtract);
-                $months = $subtract * 3;
+
-                $date->subMonths($months);
+            return $date;
-                break;
+        }
-            case 'half-year':
+        if (isset($modifierMap[$repeatFreq])) {
-                $months = $subtract * 6;
+            $subtract = $subtract * $modifierMap[$repeatFreq];
-                $date->subMonths($months);
+            $date->subMonths($subtract);
-                break;
+
-            case 'year':
+            return $date;
-            case 'yearly':
-                $date->subYears($subtract);
-                break;
         }
 
-        return $date;
+        throw new FireflyException('Cannot do subtractPeriod for $repeat_freq ' . $repeatFreq);
     }
 }
 

app/lib/FireflyIII/Shared/Toolkit/Filter.php

@@ -131,6 +131,8 @@ class Filter
     }
 
     /**
+     * @SuppressWarnings("CyclomaticComplexity") // It's exactly 5. So I don't mind.
+     *
      * @param        $range
      * @param Carbon $date
      *