Re: [PATCHv2 1/1] include: function parameter names same in declaration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Daniel P. Berrangé <berrange@xxxxxxxxxx> [2018-02-13, 09:47AM +0000]:
> On Mon, Feb 12, 2018 at 03:24:03PM -0600, Chris Venteicher wrote:
> > Headers use same function parameter names as definition code.
> > 
> > In some cases in libvirt-domain and libvirt-network an established
> > naming pattern in the header files was more consistent and informative
> > in which case the implementation was modified in the c file.
> 
> > @@ -1626,11 +1626,11 @@ int                     virDomainInterfaceStats (virDomainPtr dom,
> >   */
> >  # define VIR_DOMAIN_BANDWIDTH_OUT_BURST "outbound.burst"
> >  
> > -int                     virDomainSetInterfaceParameters (virDomainPtr dom,
> > +int                     virDomainSetInterfaceParameters (virDomainPtr domain,
> 
> Hmmmm, I kind of expected that  "dom" would be more popular than "domain",
> but I see the results are somewhat contradictory.
> 
> If we just consider the header file
> 
> $ git grep "virDomainPtr dom" include/libvirt/libvirt-domain.h  | wc -l
> 167
> $ git grep "virDomainPtr dom" include/libvirt/libvirt-domain.h  | grep "virDomainPtr domain" | wc -l
> 99
> 
> So dom==68, domain=99  => 2:3
> 
> But if we consider the source as a whole
> 
> $ git grep "virDomainPtr dom"  | wc -l
> 1863
> $ git grep "virDomainPtr dom"  | grep "virDomainPtr domain" | wc -l
> 675
> 
> So dom=1188 domain=675  => 2:1

This is biased because for a temporary variable an abbreviated name
'dom' is preferable, especially if you have an argument 'domain'.

Since function declarations serve some kind of documentation purpose,
I'd prefer the full name 'domain'. It reads so much better in my opinion
and characters are cheap.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux