2
\$\begingroup\$

The objective of this piece of code is to get a message from a cryptocurrency daemon through RPC and extract the transactions object to pass it to the view. Is there a simpler / less ugly way of doing this?

$transaction_pool = json_decode($rpc->getTransactionPool(), false);

if(isset($transaction_pool->transactions)){
    $transaction_pool = $transaction_pool->transactions;
}else{
    $transaction_pool = (object)NULL;
}

View partial:

@forelse ($transaction_pool as $transaction)
<?php $transaction_json = json_decode($transaction->tx_json); ?>
    <div class="row show-grid top-row">
        <div class="col-xs-12 col-sm-12 col-md-7">{{ $transaction->id_hash }}</div>
        <div class="col-xs-12 col-sm-7 col-md-5 pull-right">
        @if ($transaction_json->version == 2)
            <div class="col-xs-7 col-md-6"><i class="fa fa-envelope-o"></i>&nbsp;confidential</div>
        @else
            <div class="col-xs-7 col-md-6">@coin($transaction_json->amount)</div>
        @endif
            <div class="col-xs-5 col-md-6">@coin($transaction->fee)</div>
        </div>
    </div>
    @empty
    <div class="row show-grid top-row">
        <div class="col-xs-12">No transactions</div>
    </div>
@endforelse

<div class="panel-heading large">
    Transactions ({{ count($transaction_pool) }})
</div>
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! Please try to write a title that summarizes what your code does; as it is the title is quite generic and could apply to many other questions. \$\endgroup\$ – Phrancis Mar 4 '17 at 23:22
  • \$\begingroup\$ Thanks. Is this a good title? \$\endgroup\$ – Papa Lazzarou Mar 5 '17 at 15:27
  • \$\begingroup\$ Yes much better! \$\endgroup\$ – Phrancis Mar 5 '17 at 17:23
  • \$\begingroup\$ Can you give examples of JSON returned from rpc for empty and not empty cases? \$\endgroup\$ – Mike Brant May 4 '17 at 18:24
1
\$\begingroup\$

I don't get the point where you cast null to object.

I'm not sure what are you trying to achive, but you can always make if 'less ugly'.

$transcations_pool = json_decode($rpc->getTransactionPool(), false);   
$transactions_pool = isset($transactions_pool->transactions) ? 
                         $transaction_pool->transactions : 
                         (object) null;

Also i would move div .show-grid to separate file and load it like this:

@include('path/to/file', [ 'transaction_json' => json_decode($transaction->tx_json) ])

Or you can implement special method like $transaction->decodeTxJson() to handle this decoding in $transaction class.

\$\endgroup\$
  • \$\begingroup\$ Ternary operators is a big NO here. \$\endgroup\$ – Hassan Althaf Mar 5 '17 at 9:17
  • \$\begingroup\$ To be honest this is way more readable to me. Three lines to show condition, true section and false section. But i understand some people find these as you called "big NO". \$\endgroup\$ – rk-development.com Mar 5 '17 at 10:06
  • \$\begingroup\$ The object casting needs to be entirely removed. There are many clean approaches to do that. \$\endgroup\$ – Hassan Althaf Mar 5 '17 at 10:07
  • \$\begingroup\$ @rk-dev.net Righto. I kinda forgot to add the crucial part of the code. I need to count() the result of the json_decode. Since it is not Countable it returns 1 even if it is empty. Edited the question. Thanks! \$\endgroup\$ – Papa Lazzarou Mar 5 '17 at 15:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.