Re: d3dx8: Implementation of WINAPI D3DXAssembleShaderFromFileA

[Dmitry Timoshkov]

""Luis C. Busquets Pérez""  wrote:
> Changed the if clauses to make the implementation more readable.

...
> - if ( !pSrcFile )
> - return D3DXERR_INVALIDDATA;
> - else
> - {
> - len = MultiByteToWideChar( CP_ACP, 0, pSrcFile, -1, NULL, 0 );
> - pSrcFileW = HeapAlloc( GetProcessHeap(), 0, len * sizeof(WCHAR) );
> - MultiByteToWideChar( CP_ACP, 0, pSrcFile, -1, pSrcFileW, len );
> - ret=D3DXAssembleShaderFromFileW(pSrcFileW, Flags, ppConstants, ppCompiledShader, ppCompilationErrors);
> - HeapFree( GetProcessHeap(), 0, pSrcFileW );
> - return ret;
> - }
>
> }

This is not more readable than a previous version. There is no need
for 'else {}' construct and therefore for additional indentation level
at all.

d3dx8: Implementation of WINAPI D3DXAssembleShaderFromFileA

I say that it is more readable because I understood you wanted the return
code as soon as possible, but, anyway, what would you propose as code for
this function?

Would the following work for you?

+
+  if ( !pSrcFile )
+   return D3DXERR_INVALIDDATA;
+  else
+  {
+   LPWSTR pSrcFileW = NULL;
+   DWORD len;
+   HRESULT ret;
+   len = MultiByteToWideChar( CP_ACP, 0, pSrcFile, -1, NULL, 0 );
+   pSrcFileW = HeapAlloc( GetProcessHeap(), 0, len * sizeof(WCHAR) );
+   MultiByteToWideChar( CP_ACP, 0, pSrcFile, -1, pSrcFileW, len );
+   ret=D3DXAssembleShaderFromFileW(pSrcFileW, Flags, ppConstants,
ppCompiledShader, ppCompilationErrors);
+   HeapFree( GetProcessHeap(), 0, pSrcFileW );
+   return ret;
+  }
}
> This is not more readable than a previous version. There is no need
> for 'else {}' construct and therefore for additional indentation level
> at all.
>
> --
> Dmitry.

d3dx8: Implementation of WINAPI D3DXAssembleShaderFromFileA

[Michael Stefaniuc]

[e-mail] wrote:
> I say that it is more readable because I understood you wanted the return
> code as soon as possible, but, anyway, what would you propose as code for
> this function?
>
> Would the following work for you?

Just go for
if ( !pSrcFile )
   return D3DXERR_INVALIDDATA;

LPWSTR pSrcFileW = NULL;
DWORD len;
HRESULT ret;
...

Like Dimitry said the "else" is not needed at all.

bye
    michael
> -
> - if ( !pSrcFile )
> - return D3DXERR_INVALIDDATA;
> - else
> - {
> - LPWSTR pSrcFileW = NULL;
> - DWORD len;
> - HRESULT ret;
> - len = MultiByteToWideChar( CP_ACP, 0, pSrcFile, -1, NULL, 0 );
> - pSrcFileW = HeapAlloc( GetProcessHeap(), 0, len * sizeof(WCHAR) );
> - MultiByteToWideChar( CP_ACP, 0, pSrcFile, -1, pSrcFileW, len );
> - ret=D3DXAssembleShaderFromFileW(pSrcFileW, Flags, ppConstants,
>
> ppCompiledShader, ppCompilationErrors);
> - HeapFree( GetProcessHeap(), 0, pSrcFileW );
> - return ret;
> - }
>
> }
> > This is not more readable than a previous version. There is no need
> > for 'else {}' construct and therefore for additional indentation level
> > at all.
> >
> > --
> > Dmitry.

d3dx8: Implementation of WINAPI D3DXAssembleShaderFromFileA

[Frank Richter]

On 27.11.2007 09:28, Michael Stefaniuc wrote:
> Just go for
> if ( !pSrcFile )
>    return D3DXERR_INVALIDDATA;
>
> LPWSTR pSrcFileW = NULL;
> DWORD len;
> HRESULT ret;
> ...

That's C++, not C, isn't it?

-f.r.

d3dx8: Implementation of WINAPI D3DXAssembleShaderFromFileA

[Michael Stefaniuc]

Frank Richter wrote:
> On 27.11.2007 09:28, Michael Stefaniuc wrote:
> > Just go for
> > if ( !pSrcFile )
> >    return D3DXERR_INVALIDDATA;
> >
> > LPWSTR pSrcFileW = NULL;
> > DWORD len;
> > HRESULT ret;
> > ...
>
> That's C++, not C, isn't it?

No it is C as C99 allows to have the variable definitions in the middle
of the code. But Wine itself doesn't allow that and i have missed that.

bye
    michael