Re: d3dx8: Implementation of WINAPI D3DXAssembleShaderFromFileA

[Dmitry Timoshkov]

""Luis C. Busquets Pérez"" <luis.busquets(a)ilidium.com> 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.

-- 
Dmitry.

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]

luis.busquets(a)ns1.ilidium.com 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.

-- 
Michael Stefaniuc               Tel.: [phone]
Sr. Network Engineer            Fax.: [phone]
--------------------------------------------------------------------
Reg. Adresse: Red Hat GmbH, Hauptstätter Strasse 58, 70178 Stuttgart
Handelsregister: Amtsgericht Stuttgart HRB 153243
Geschäftsführer: Brendan Lane, Charlie Peters, Michael Cunningham,
                 Werner Knoblich

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
-- 
Michael Stefaniuc                           Tel.: [phone]
Consulting Communications Engineer          Fax.: [phone]
--------------------------------------------------------------------
Reg. Adresse: Red Hat GmbH, Hauptstätter Strasse 58, 70178 Stuttgart
Handelsregister: Amtsgericht Stuttgart HRB 153243
Geschäftsführer: Brendan Lane, Charlie Peters, Michael Cunningham,
                 Werner Knoblich