c++ - What really makes this code bad -


at company came across following 2 code snippets, found thoroughly unpleasant @ first sight, in spirit of offering constructive feedback engineer wrote that, i'm trying come technical arguments why code bad:

filetableentry * filermanager::getfiletableentry(uint32_t i) const {   return &(getfiletable()[i]); }  … (uint32_t = 0; < container_.size(); ++i) {    *getfiletableentry(i) = filetableentry();    // getfiletableentry accesses std::vector in filemanager } 

my main arguments :

  1. this code indirect , misleading, should not use getter initiliaze (part of filemanager), @ least setter: being more direct makes code easier understand.
  2. the getter leaks internal state of filemanager, encapsulation filemanager means nothing @ point. worse, getter promises applicable const objects, merrily used mutate internal state of filemanager. breaking encapsultion sure path making refactorings harder.

are there other arguments not writing code missing??

another argument against code signature of getfiletableentry returns pointer in situations when object present. calls reference, not pointer:

filetableentry& filermanager::getfiletableentry(uint32_t i) const {     return getfiletable()[i]; } 

this should address first point. second point can addressed making reference const:

const filetableentry& filermanager::getfiletableentry(uint32_t i) const {     return getfiletable()[i]; } 

this prohibits callers modifying internal state returned getfiletableentry.

note: make getfiletableentry function useful, 1 should add bounds check of index passed in catch errors early.


Comments

Popular posts from this blog

toolbar - How to add link to user registration inside toobar in admin joomla 3 custom component -

linux - disk space limitation when creating war file -