PDA

View Full Version : Best practice - first class


rocketmcm
03-23-2010, 09:23 PM
So I am working on a project and may start implementing an OOP approach if I have time.

So I spent some time doing a little bit of look around and have manages to scramble this together which seems to be working.

Just wondering how is this for an approach, is this the way I should be doing it, I have just stopped at something which I have got working and perhaps this is not the best way.

Any help would be grateful, cheers!

package rm.math
{
public class RandomNumber
{

public function RandomNumber()
{
}

public static function randomNumberRange(max:Number, min:Number = 0, rounded:Boolean = true):Number{
return rounded ? Math.round(Math.random()*(max-min)+min) : Math.random()*(max-min)+min;
}

}
}

wvxvw
03-24-2010, 02:11 AM
Well, it is rather an interesting situation because:
In general it is a good practice to put common functionality in one place, so, the intention was good.
But, Math.random() isn't the fastest thing in the world, and AS3 compilers don't do inlining - they've no idea what it is. Add to that the static field access is expensive, function call is even more expensive. So, if this function is going to be used extensively, it would be better to "inline" it by hand.
Also, you can make it faster, depending on the particular situation, like, for instance, working with integers is always faster then working with floating point numbers.
If the situation allows it, casting to int is faster then Math.round().

Regarding best practices of formatting and overall code style:
- You must call super() in constructor on the first line.
- you must name the package following this rule:
<3rd level domain>.<2nd level domain>.<project>.[library].[sub-library]
Example:
com.rocketmcm.best.practices.utils

Operators that accept two operands should be delimited by spaces, that is add, multiply, assign, divide and so on should spell like this:
X + Y, X * Y, X = Y
While operators that accept single operand should not be delimited by space, example:
!X, -X, ~X
You'd also be on the safe side, if you won't write lines of code longer then 80 characters. Historically, this is the width of the screen, however, this is similar to older unwritten standards for typography. Normally, the line longer then 72 characters is said to be to long and difficult to read.

rocketmcm
03-24-2010, 12:53 PM
Great, thanks very much for your info.

Had a quick google about inlining as I wasn't sure, its when the compiler replaces the function call with the whole function to cut down the time spent on function calling? Wiki: http://en.wikipedia.org/wiki/Inline_function

So you would suggest this instead:
int(Math.random() * (max - min) +min)

I ran a highly unscientific test using Grant Skinners performance test
private function randomRound():void {
var i:int = 0;
for(i=0; i<10000; i++){
var j:int = Math.round(Math.random() * (1000 - 1) +1);
}
}

private function randomInt():void {
var i:int = 0;
for(i=0; i<10000; i++){
var j:int = int(Math.random() * (1000 - 1) +1);;
}
}

[MethodTest name='Random Round' time=2.8 min=1 max=5 deviation=1.428 memory=0]
[MethodTest name='Random Int' time=2.7 min=1 max=3 deviation=0.751 memory=0]

I am sure you can find some problems with my test though.

Thanks very much for your help!

wvxvw
03-24-2010, 10:40 PM
Yes, inlining is exactly what you quoted.
Well, regarding the test, you should really consider debug / release compilation and player. Normally, you'd want to test against release player, release compilation since this is what the end user is much likely will have. Release compile has quite a bit of debugging info stripped of, thus, for example, method calls will be faster (they won't include debug info related to the AS file location and line number) etc.
Speaking of performance, FlashPlayer for unknown reason always treats numeric constants as float point numbers, so, casting results of math operations to integers usually helps optimization a bit... However, this also depends on a target platform and the player version, and even on the specific user hardware...
There's an interesting article regarding this subject:
http://jpauclair.net/2010/03/15/flash-asm/

cpucpu
03-25-2010, 02:55 AM
What exactly are the implications of writting lines longer than 80 characters?
I belived that it was for printing purposes so that printer code looks exactly like screen code, an also for projecting purposes i think perhaps something to do with resolutions or display format(4:3, 6:9, ...).

Personally i don't have nay dificult reading code at 100+ lines if that doesn't occurs often.
e.g.
var myTween:GenericActuator = Actuate.tween(_theSoundTransform, seconds, { volume: volumeLevel / 100 } );

But for that "incompatibility" issues, or standards i keep aware of that.

wvxvw
03-25-2010, 12:12 PM
1. It stretches the forum layout :D
2. Until very recently, in the office, I had a 14" monitor - these long lines make the code editor scroll.
3. If you want to print that in a book, you'll make it complicated for the editor to preserve your code as is.

Besides, "can read" and "is comfortable to read" isn't the same thing ;)

ASWC
03-25-2010, 12:46 PM
1. It stretches the forum layout :D
2. Until very recently, in the office, I had a 14" monitor - these long lines make the code editor scroll.
3. If you want to print that in a book, you'll make it complicated for the editor to preserve your code as is.

Besides, "can read" and "is comfortable to read" isn't the same thing ;)So best coding practice: Don't write 80 characters + of code since that messes up the actionscript.org forum display and it doesn't fit on wvxvw 14" monitor ....

This best coding practice is gonna be hard to sell ... :p

rocketmcm
03-25-2010, 02:27 PM
Yes, inlining is exactly what you quoted.
Well, regarding the test, you should really consider debug / release compilation and player. Normally, you'd want to test against release player, release compilation since this is what the end user is much likely will have. Release compile has quite a bit of debugging info stripped of, thus, for example, method calls will be faster (they won't include debug info related to the AS file location and line number) etc.
Speaking of performance, FlashPlayer for unknown reason always treats numeric constants as float point numbers, so, casting results of math operations to integers usually helps optimization a bit... However, this also depends on a target platform and the player version, and even on the specific user hardware...
There's an interesting article regarding this subject:
http://jpauclair.net/2010/03/15/flash-asm/

Hmm looks like you are always fighting a losing battle. I suppose you just got to go for just damage limitation.

Truth be told I am only using this function once in my code and I just fancied to see if I could get it working. Plus it lets my lecturer see I am thinking outside the box a little more than others. Life is a competition:mad:

wvxvw
03-25-2010, 02:42 PM
Well, now I have 24" inch monitor, but I have project explorer, outline and output panels open along with the code editor, so, long lines are still not OK :)
Besides, oh comon, it's really difficult to read long lines! Why make yourself suffer? Best practices are about making it easier for an average human being to deal with your code, but if you are an opposite of an average, then, none of the rules apply :)

cpucpu
03-26-2010, 12:25 AM
So we are down to lines too long because of they:
-may intefere with other screen stuff like code panels.
-are hard to read.
-have to be edited in order to print right (less common)
-people that have monitors with less monitor resolution than us can have issues with lines (i've seen this before, non necesarly because i use and old monitor, but rather because of they use very non-standard big resolutions)

At least i, with my 12 pixels consolas and a 16:9 monitor. 80 chars (including tabs) feels a bit narrow. I am more confortable with up to about 100 chars on the longest lines, knowing that's not too much, not hard to read, not many space wasted, and even considering that there stills being plenty blank space as i don't use any right panels. But i don't really know whether this long can have isues when printing, even if they use small font sizes.

DougyTheFreshmaker
03-26-2010, 08:54 AM
So I am working on a project and may start implementing an OOP approach if I have time.




package rm.math
{
public class RandomNumber
{

public function RandomNumber()
{
}

public static function randomNumberRange(max:Number, min:Number = 0, rounded:Boolean = true):Number{
return rounded ? Math.round(Math.random()*(max-min)+min) : Math.random()*(max-min)+min;
}

}
}

Because this is such a basic piece of code, it's difficult to critique anything but minor issues, such as stylistic concerns, which are debatable ad infinitum. In other words, the responses are worth considering, but remember that they are, this one included, mostly opinion.

About the actual code, I would first say that this is not a good demonstration of OOP, if that was the intent (I'm unclear on this), due to the fact that the class consists of only a static function and nothing else. I agree with this decision, though, as the function should be static since it needn't rely on any state. The point, though, is that if you're looking for an evaluation of your OOP/OOD approach, this code won't help that end.

As for the function, it does too much and there is duplicate code. In this case, the offending code is:
Math.random()*(max-min)+min
And as it turns out, this code reflects what your function actually claims to do. The branching and rounding part is extra, unspecified behavior. You can simply round the number returned from the Math.random()*(max-min)+min function or, if you feel it is necessary, write a new function such as randomIntRange(...) or randomNumberRangeRounded (etc) which returns the expected value. If you need to easily swap between the two at runtime, you can use an interface, function pointer or anonymous type to do so.

A common misconception of the "rounded" algorithm presented is that it produces a uniform distribution of random integers. This is not the case, as "min" and "max" will be generated half as much as the numbers "inside of" the interval. It may also be worth noting that the interval generated by the non-rounded version is [min, max). That is, min may be selected exactly, but max never will be (This adds an additional, very small skew to the results of both functions). The suggestion of "casting to int" yields its own unique results. It was qualified with "if the situation allows", which is important. Truncation by casting is different from rounding, which is different from flooring, etc. You may be aware of these issues already.

And some stylistic opinions. Remeber, these are opinions in the extreme!

This class should not be instantiated, but there is no (language level) support for preventing instantiation. The compiler will generate a constructor itself, but by explicitly writing one, it would imply to me that the class is supposed to be instantiated at some point.

I tend to avoid defaulting parameters. Explicitly passing in a min only makes code clearer, and if the min were called with zero enough, another function could be written to proxy the call instead. Functions which would benefit most from defaulted parameters have a high number of parameters, and this is generally considered a bad thing. Additionally, the effectiveness of defaulting the parameters depends on them being ordered in the frequency that they are needed, and so this is inadequate when the frequency is unknown or in siutations in which the "last" parameter need be specified, but the other ones don't, etc. The easy counter to my argument is that the number of functions with different names may grow rapidly when avoiding default arguments, and the main solution to this problem, function overloading, is not available in AS3--so that's worth considering as well.

Last and least, I use 80 characters as my width. I do code from the console on occasion. In high-resolution environments, however, I can fit a number of files on the screen horizontally given lower "width" requirements (2 for 80, 3 for 60 on this screen with the chosen font). I would consider 80 "safe" if the code is to be developed or read by multiple parties.