Go Back   vb.org Archive > vBulletin 4 Discussion > vB4 Programming Discussions

Reply
 
Thread Tools Display Modes
  #1  
Old 03-16-2011, 01:18 PM
Boofo's Avatar
Boofo Boofo is offline
 
Join Date: Mar 2002
Location: Des Moines, IA (USA)
Posts: 15,776
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default Is this proper coding?

Can someone tell me in their opinion if this is proper coding? The following code is the fix for the division by zero error.

I am replacing this:

Code:
if ($totalthreads == 0)
{
    $totalthreads = 1;
}
if ($totalposts == 0)
{
    $totalposts = 1;
}

With this:

Code:
$totalthreads == 0 ? $totalthreads = 1 : $totalthreads; 
$totalposts == 0 ? $totalposts = 1 : $totalposts;

It works, but I am curious as to what your feelings are as to whether this is actually a proper way to do it or not.

Can anyone tell I got bored enough to do this?
Reply With Quote
  #2  
Old 04-14-2011, 12:33 PM
Boofo's Avatar
Boofo Boofo is offline
 
Join Date: Mar 2002
Location: Des Moines, IA (USA)
Posts: 15,776
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

No opinions one way or the other?
Reply With Quote
  #3  
Old 04-14-2011, 04:06 PM
Lynne's Avatar
Lynne Lynne is offline
 
Join Date: Sep 2004
Location: California/Idaho
Posts: 41,180
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

I don't know which is better. When I 'read' code, I like it better the first way - it just looks cleaner to me.
Reply With Quote
  #4  
Old 04-14-2011, 04:22 PM
Boofo's Avatar
Boofo Boofo is offline
 
Join Date: Mar 2002
Location: Des Moines, IA (USA)
Posts: 15,776
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

I agree somewhat that it is easier to understand what the code in the first example is doing. But I also like the second way as it condenses 8 lines into 2.
Reply With Quote
  #5  
Old 04-14-2011, 04:30 PM
Lynne's Avatar
Lynne Lynne is offline
 
Join Date: Sep 2004
Location: California/Idaho
Posts: 41,180
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

For single 'actions' to a condition, I actually like it like this:

PHP Code:
if ($totalthreads == 0$totalthreads 1
if (
$totalposts == 0$totalposts 1
Reply With Quote
  #6  
Old 04-14-2011, 04:49 PM
Boofo's Avatar
Boofo Boofo is offline
 
Join Date: Mar 2002
Location: Des Moines, IA (USA)
Posts: 15,776
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

Yes, that would be the better way to do it for single actions. I think I will use that instead of my way since I don't really need the else part of it for this particular code. Thanks for pointing that out.

I guess I'm just used to the brackets being there.
Reply With Quote
  #7  
Old 04-19-2011, 07:59 AM
Disasterpiece's Avatar
Disasterpiece Disasterpiece is offline
 
Join Date: Apr 2007
Location: GER
Posts: 765
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

I can trump that.
PHP Code:
if (!$totalthreads$totalthreads 1
if (!
$totalposts$totalposts 1
PHP Code:
$totalthreads max($totalthreads,1); 
$totalposts max($totalposts,1); 
Reply With Quote
  #8  
Old 04-19-2011, 10:27 AM
Boofo's Avatar
Boofo Boofo is offline
 
Join Date: Mar 2002
Location: Des Moines, IA (USA)
Posts: 15,776
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

Quote:
Originally Posted by Disasterpiece View Post
I can trump that.
PHP Code:
if (!$totalthreads$totalthreads 1
if (!
$totalposts$totalposts 1
PHP Code:
$totalthreads max($totalthreads,1); 
$totalposts max($totalposts,1); 
Will you explain the logic behind that? Also, how would you do the following using max?

Code:
if ($vbulletin->options['something_here'] == '') $comma_separated = '0';
Reply With Quote
  #9  
Old 04-19-2011, 11:35 AM
YankForum's Avatar
YankForum YankForum is offline
 
Join Date: Mar 2010
Location: MY
Posts: 304
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

Quote:
Originally Posted by Disasterpiece View Post
I can trump that.
PHP Code:
if (!$totalthreads$totalthreads 1
if (!
$totalposts$totalposts 1
PHP Code:
$totalthreads max($totalthreads,1); 
$totalposts max($totalposts,1); 
nice one , but the most important question here is which one will be processed faster in php? in my opinion the second proposed code by boofo should be the fastest since it only takes 2 values true or false??? of course as lynne says the first one is more readable and easy to customize , i like it the most
nice thread , thanks boofo
Reply With Quote
  #10  
Old 04-19-2011, 01:36 PM
Disasterpiece's Avatar
Disasterpiece Disasterpiece is offline
 
Join Date: Apr 2007
Location: GER
Posts: 765
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default

Quote:
Originally Posted by Boofo View Post
Will you explain the logic behind that? Also, how would you do the following using max?

Code:
if ($vbulletin->options['something_here'] == '') $comma_separated = '0';
in php you have automatic typecast, so everything evalueates to either true or false. Zero (numeric) evaluates to false, so the ! inverts the evaluation to "true" and therefore you set the totalthreads to 1. It's just that it looks nicer because it's short :P
if it takes one or more additional cycles to process it? maybe. But it's php after all, so the uber-performance isn't that of an issue.

max is a mathematic function which returns the greater value of the given params. If you don't want to have a number which is negative or equal to 0, you use max($n, 1) so you guarantee that you always have at least 1.
Not sure about the performance, It's possible that the php engine uses the native c math function, so this might even speed things up a bit? dunno but then again, doesn't really matter :P

Just intended to brag with my mad php skillz, honestly I think the idea itself is an issue here (it throws an error when it's zero? well, just increase it then. <- wtf?)
If you really want to get help, you should explain your problem instead of the solution


PHP Code:
if ($vbulletin->options['something_here'] == ''$comma_separated '0'
I have nothing to add. maybe use "empty()" instead of testing for empty string, since the empty function checks for null as well and doesn't throw a warning when the value is not set.
Reply With Quote
Reply

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT. The time now is 02:42 PM.


Powered by vBulletin® Version 3.8.12 by vBS
Copyright ©2000 - 2024, vBulletin Solutions Inc.
X vBulletin 3.8.12 by vBS Debug Information
  • Page Generation 0.04540 seconds
  • Memory Usage 2,272KB
  • Queries Executed 11 (?)
More Information
Template Usage:
  • (1)SHOWTHREAD
  • (1)ad_footer_end
  • (1)ad_footer_start
  • (1)ad_header_end
  • (1)ad_header_logo
  • (1)ad_navbar_below
  • (1)ad_showthread_beforeqr
  • (1)ad_showthread_firstpost
  • (1)ad_showthread_firstpost_sig
  • (1)ad_showthread_firstpost_start
  • (4)bbcode_code
  • (8)bbcode_php
  • (3)bbcode_quote
  • (1)footer
  • (1)forumjump
  • (1)forumrules
  • (1)gobutton
  • (1)header
  • (1)headinclude
  • (1)navbar
  • (3)navbar_link
  • (120)option
  • (1)pagenav
  • (1)pagenav_curpage
  • (1)pagenav_pagelink
  • (10)post_thanks_box
  • (10)post_thanks_button
  • (1)post_thanks_javascript
  • (1)post_thanks_navbar_search
  • (10)post_thanks_postbit_info
  • (10)postbit
  • (10)postbit_onlinestatus
  • (10)postbit_wrapper
  • (1)spacer_close
  • (1)spacer_open
  • (1)tagbit_wrapper 

Phrase Groups Available:
  • global
  • inlinemod
  • postbit
  • posting
  • reputationlevel
  • showthread
Included Files:
  • ./showthread.php
  • ./global.php
  • ./includes/init.php
  • ./includes/class_core.php
  • ./includes/config.php
  • ./includes/functions.php
  • ./includes/class_hook.php
  • ./includes/modsystem_functions.php
  • ./includes/functions_bigthree.php
  • ./includes/class_postbit.php
  • ./includes/class_bbcode.php
  • ./includes/functions_reputation.php
  • ./includes/functions_post_thanks.php 

Hooks Called:
  • init_startup
  • init_startup_session_setup_start
  • init_startup_session_setup_complete
  • cache_permissions
  • fetch_threadinfo_query
  • fetch_threadinfo
  • fetch_foruminfo
  • style_fetch
  • cache_templates
  • global_start
  • parse_templates
  • global_setup_complete
  • showthread_start
  • showthread_getinfo
  • forumjump
  • showthread_post_start
  • showthread_query_postids
  • showthread_query
  • bbcode_fetch_tags
  • bbcode_create
  • showthread_postbit_create
  • postbit_factory
  • postbit_display_start
  • post_thanks_function_post_thanks_off_start
  • post_thanks_function_post_thanks_off_end
  • post_thanks_function_fetch_thanks_start
  • post_thanks_function_fetch_thanks_end
  • post_thanks_function_thanked_already_start
  • post_thanks_function_thanked_already_end
  • fetch_musername
  • postbit_imicons
  • bbcode_parse_start
  • bbcode_parse_complete_precache
  • bbcode_parse_complete
  • postbit_display_complete
  • post_thanks_function_can_thank_this_post_start
  • pagenav_page
  • pagenav_complete
  • tag_fetchbit_complete
  • forumrules
  • navbits
  • navbits_complete
  • showthread_complete