API REVIEW REQUEST: Public API for Backgrounds and Borders on Region

[Tom Schindl]

Am 27.08.12 16:25, schrieb Richard Bair:
> 
> On Aug 26, 2012, at 11:07 PM, Tom Schindl wrote:
> 
>> Am 26.08.12 23:12, schrieb Jonathan Giles:
>>> Rich,
>>>
>>> The API looks fine. I only have small comments about minor issues:
>>>
>>> * I agree with John - it is worth reviewing the hashCode code. I'm not
>>>   sure it is worth pre-computing everything given the cost of that,
>>>   but there is a risk that you'll (very infrequently) end up with a
>>>   hashCode for an object actually being zero, and in this case you'll
>>>   always be running the slow path through hashCode(). Perhaps you can
>>>   simply introduce a boolean to note whether hashCode has been computed.
>>> * In BorderRadii, I'm not overly familiar with the terminology of some
>>>   of the methods in this class. For example,
>>>   getTopLeftVerticalRadius(). I wonder if the JavaDoc can be more
>>>   useful than "The length of the vertical radii of the top-left
>>>   corner."? Of course, I could just become more well-versed in the CSS
>>>   specifications :-)
>>> * In BorderRadii, I understand that the radii can be interpreted as
>>>   either a value or a percentage, but I'm not fond of the method name
>>>   you use to expose this, e.g. isTopLeftVerticalRadiusAsPercentage().
>>>   It is the 'As' that bothers me - I would have rather had an 'A', but
>>>   I can see why you chose to use 'As' - so that we don't end up with
>>>   isTopLeftVerticalRadiusAPercentage() (which is actually less
>>>   readable, even if it is better English). Personally, I would still
>>>   use 'A' rather than 'As', but I'm not going to pursue it any further
>>>   as it is a gut feel choice (and you've used 'As' throughout the
>>>   Region API).
>>
>> Why not name if getTopLeftVerticalRadiusType() and return there an enum?
>> I guess there are more values who can have an absolute and percentage
>> value so they'd all use the same enum.
> 
> I think it would only be a two-valued enum, in which case what advantage does it have over a boolean? I believe that besides percentage, all other CSS values are reduced to an actual literal value in the end. Even "em" based computations are done by the CSS engine at the time that the value is looked up in the CSS engine.
> 

If there's going to be another one in future you could add it - not sure
this is binary compatible and needed - but you are closing the door with
this naming already a bit not?

Tom


-- 
B e s t S o l u t i o n . a t                        EDV Systemhaus GmbH
------------------------------------------------------------------------
tom schindl                 geschäftsführer/CEO
------------------------------------------------------------------------
eduard-bodem-gasse 5-7/1   A-6020 innsbruck     fax      ++43 512 935833
http://www.BestSolution.at                      phone    ++43 512 935834