Hi Semyon,
On 26/09/2017 21:58, Semyon Sadetsky wrote:
Hi Alexey,
On 9/26/2017 12:29 PM, Alexey Ivanov wrote:
Hi Semyon,
ShellFolder2.cpp
944 pIcon->Extract(szBuf, index, &hIcon, *0*, sz);
I think 0 should really be |NULL|.
Ok, but both represent the null pointer in Win32.
Yes, I know. Yet NULL makes it clear that it's a null-pointer rather
than an index, size…
It's still zero in the latest review.
The result of the call is ignored now. Is it intentional?
Yes, it has been working the same way before the fix. The function
returns the Icon reference which is 0 in case of OS API call error.
Win32ShellFolder2.java
1010 private static Image makeIcon(long hIcon, int bsize) {
|bsize| was called |baseSize| previously, and it conveyed the meaning
better, didn't it?
1043 if(hIcon <= 0) {
1044 if (isDirectory()) {
1045 return getShell32Icon(4, size);
1046 } else {
1047 return getShell32Icon(1, size);
1048 }
I guess I understand what hides behind 4 and 1: generic folder and
generic file icon ids. Would declaring these numbers as constants
improve readability?
Ok.
|getIcon(int size)| and |getIcon(boolean getLargeIcon)| are somewhat
similar. The latter provides additional caching. Can it be simplified
to re-use portions of new |getIcon(int size)|?
It has additional difference because of query for a fixed icon size
from another API call. It's better to leave it as it is.
Okay.
Regards,
Alexey
http://cr.openjdk.java.net/~ssadetsky/8182043/webrev.02/
--Semyon
Regards,
Alexey
On 22/09/2017 22:05, Semyon Sadetsky wrote:
<SNIP>
