What is the benefit of this change?

@brianteeman turn explicit the meaning of this number on code. I guess it's much more easy to read in that way.

Seems harder to read to me

@brianteeman do you think that the number itself is "self-explanatory"?

Speaking purely personally - yes

@brianteeman then should I close this one?

Maybe others agree with you

I think it is easier to read. It is hard to know that 1048576 is 1MB

I don't think you need to define it as a constant to give it meaning. It's just as easy to put a comment above the line saying "convert to megabytes, 1048576 bytes = 1MB" (or something like that).

You youngsters - i see that number and automatically know its 1mb ;)

On 27 January 2016 at 12:34, Michael Babker notifications@github.com
wrote:

I don't think you need to define it as a constant to give it meaning. It's
just as easy to put a comment above the line saying "convert to megabytes,
1048576 bytes = 1MB" (or something like that).

—
Reply to this email directly or view it on GitHub
#8994 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@mbabker constants make it easy to change, as change in one line. also, I want to avoid put unnecessary comments if I can explain it just in code, and this number appear twice on this code, so I feel bad to comment it 2 times.

But its never going to change

On 27 January 2016 at 12:37, Jefersson Nathan notifications@github.com
wrote:

@mbabker https://github.com/mbabker constants make it easy to change,
as change in one line. also, I want to avoid put unnecessary comments if I
can explain it just in code, and this number appear twice on this code, so
I feel bad to comment it 2 times.

—
Reply to this email directly or view it on GitHub
#8994 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@brianteeman

Defining it as a class constant adds the constant as a part of the public API and assumes that value is in some way expected to be used outside JProfiler. And by the time you've added the full doc block for the constant you've added more lines to the file (see JVersion::PRODUCT as an example) than just simple one line comments in two places. As for things changing, doubtful; changing units would be a B/C break (the redesigned profiler in the Framework doesn't even do conversions but just stores all values in bytes and lets the user convert if they choose).

@mbabker ONE_MEGABYTE will never change, if you want to change the value you should create another constant as TWO_MEGABYTE, and for BC break, it'll happens anyway if the value is changed. I see no problem with dock block as it'll not duplicate things (comments). I see no point to make it less clear.

It's crystal clear regardless of whether it's a class property, constant,
or a comment. There is no need to make properties and constants out of
arbitrary values that are used in one or two places. If there were
numerous references to the value across the CMS, it'd be a different
argument. But your proposal is seriously equating to "we use this integer
twice in this class, define it as a constant to give it some sort of extra
meaning".

On Wednesday, January 27, 2016, Jefersson Nathan notifications@github.com
wrote:

@mbabker https://github.com/mbabker ONE_MEGABYTE will never change, if
you want to change the value you should create another constant as
TWO_MEGABYTE, and for BC break, it'll happens anyway if the value is
changed. I see no problem with dock block as it'll not duplicate things
(comments). I see no point to make it less clear.

—
Reply to this email directly or view it on GitHub
#8994 (comment).

Thanks @mbabker and @brianteeman for the fast answers. I'm closing it right now