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\$
3
  • 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, 2017 at 23:22
  • \$\begingroup\$ Yes much better! \$\endgroup\$
    – Phrancis
    Mar 5, 2017 at 17:23
  • \$\begingroup\$ Can you give examples of JSON returned from rpc for empty and not empty cases? \$\endgroup\$
    – Mike Brant
    May 4, 2017 at 18:24

1 Answer 1

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\$
4
  • \$\begingroup\$ Ternary operators is a big NO here. \$\endgroup\$
    – codez
    Mar 5, 2017 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\$
    – rkd.me
    Mar 5, 2017 at 10:06
  • \$\begingroup\$ The object casting needs to be entirely removed. There are many clean approaches to do that. \$\endgroup\$
    – codez
    Mar 5, 2017 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\$ Mar 5, 2017 at 15:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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