What’s wrong with this code #4 – discussion…


I've decided to use "discussion" rather than "answer" on these.

So, in reference to the last question:

First, thanks to all who posted. The discussions in the comments are always interesting to read, and I usually learn something from them.

I was going for the call to GC.Collect(). The intention of the user was to clean up the NetworkResponse objects that came back from the call to GetNetwork().

This is almost always the wrong thing to do. Rico does a far better job discussing it here than I could.

So, that's what I was going for, but it looks like I'll need to take a bit more care in creating my examples, to limit the possibilities a bit.

To expand a bit on some of the comments:

There was a fair amount of discussion about resource leaks. Given what I wrote, there probably isn't enough information to determine whether a resource leak is an issue.

There was also a lot about data validation. Data validation is important when you're writing API routines and/or if you have security concerns, but my personal choice would be not to add data validation to this routine, because the runtime does a pretty good job in most cases. If, for example, the ArrayList is null, then the foreach loop is going to throw an exception without me checking.

The disadvantage of that approach is that the exception may not be quite as informative as a hand-coded one.

Hope that makes sense. 

 

Comments (15)

  1. Darren Oakey says:

    re: datavalidation

    as you are probably painfully aware, I always have a problem with nulls – and think that it is _very_ important that routines like this validate nulls.

    Why? As you say – in this case, nulls just cause an exception don’t they.

    sorta. The trouble with nulls is that the errors that eventually raise their heads show up in a place which has no relation to the original source of the problem…

    For instance… suppose I have my new Employee( string firstName, string lastName) function. And suppose I don’t validate the values aren’t null – I just store them away.

    So, we have a bug in our input routines – they mistakenly send a null in for the last name.

    Everything works fine for a long time, and then our report function falls over with a null pointer exception. We look at the line. It’s something like serialized = a + b + c + d + e.firstName + e.lastName + f + g;

    So – we have something falling over, and absolutely no idea what’s wrong – there is nothing about the error message, the stack trace or anything that even hints that the error is coming from an employee – let alone that it’s a failure of the validation routines in the dodgy NewFormQuickCustomerCreation.

    Even with a simple function like this:

    void Blah( string x )

    {

    DoSomethingWith( x );

    }

    we really don’t know what’s happening inside DoSomethingWith – and they just might be doing something with null that we aren’t expecting, like putting it in some static variable for failure later!

    In the old VB days, I had routines which gave me a stack trace with the complete values of the parameters to the functions at each level, which helped a lot (almost like a unix style core dump) – but with .NET – if you get notified that a Null Reference exception has occurred in production, you basically have buckleys chance of tracking it down – you just have to try to reproduce it, or pretend it didn’t happen and look confused when the users ring and say it happened again.

    So – I make it a rule anywhere I go that EVERY variable in EVERY public function MUST be checked for null, if you aren’t expecting a null… because then at least the exception occurs close to where the problem occurred. It’s just too dangerous otherwise – the software is completely unsupportable.

    on a side note (and this will horrify many, but I think it’s funny) at home I’ve renamed my nonnull template to _ – which leads to much cleaner code…

    _<string> Add( _<string> a, _<string> b) {return _<string>.Safe( a.value+b.value);}

    A small price to pay for 100% guarantee I’ll never have this sort of error in my programs!

  2. That’s like a police officer saying "oh, I’ll let that criminal go, because I know that he will be caught by someone right over there". It just doesn’t sit well with me.

    Rather, I think it does bring up a good argument for method interception (NOT AOP), being able to say attribute certain parameters to have the runtime perform certain checks, or allowing me to perform some other kind of pre-processing (and throwing an exception as a result).

  3. AndrewSeven says:

    We could also say that it’s like the law not needing to say theft of a toaster because it is covered by theft less than $1000

  4. James Curran says:

    Actually, I’d say it’s like a trapeze artist having one net, instead of three….

  5. Dan Golick says:

    Thank-you

    I’ve been advocating letting the CLR throw exceptions on null references when not at the boundaries of the code. When I’m checking user input or checking correct usage of a library I want to throw a tailored exception but internally I’ll let the CLR checking give me my exceptions.

  6. dd says:

    http://transcom.linkdata.cn http://nieheji.linkdata.cn http://diet.linkdata.cn http://2ndhouse.linkdata.cn http://sensor.linkdata.cn http://jiajiao.linkdata.cn http://gxjiameng.linkdata.cn http://gxliansuo.linkdata.cn http://gxstore.linkdata.cn http://gxmachine.linkdata.cn http://washstore.linkdata.cn http://investment.linkdata.cn http://www.bjdstv.com http://www.afafw.com/ http://www.chinafire.com/ http://www.chinafire.com/cn/product/fire/index.cfm http://zlyy.linkdata.cn http://ykyy.linkdata.cn http://yiyao.linkdata.cn http://yyzs.linkdata.cn http://hengqi.linkdata.cn http://flzx.linkdata.cn http://tm.linkdata.cn http://bqj.linkdata.cn http://www.speec.com.cn/raggings-2.html http://www.speec.com.cn/sheaves.html http://www.shsaige.com/ http://gongyipin.linkdata.cn http://golf.linkdata.cn http://denglizi.linkdata.cn http://painting.linkdata.cn http://trip.linkdata.cn http://hunjie.linkdata.cn http://www.168777.com/ http://caixin.linkdata.cn http://zhengrong.linkdata.cn http://lubricant.linkdata.cn http://runhuazhi.linkdata.cn http://dinuan.linkdata.cn http://xinjiangtrip.linkdata.cn http://daliantrip.linkdata.cn http://exchanger.linkdata.cn http://jtdh.linkdata.cn http://coolequip.linkdata.cn http://aircondition.linkdata.cn http://ywkg.linkdata.cn http://www.jiaxinol.com/ http://www.ejing.net/shouye/ http://www.shuntu.net/ http://ggths.linkdata.cn http://additive.linkdata.cn http://elevator.linkdata.cn http://paperbag.linkdata.cn http://shadcompany.linkdata.cn http://printcompany.linkdata.cn http://addesign3.linkdata.cn http://sextool.linkdata.cn http://tuozhansh.linkdata.cn http://changhaihospital.linkdata.cn http://tuopan.linkdata.cn http://mobilering.linkdata.cn http://pctrain.linkdata.cn http://developzone.linkdata.cn http://www.shczzx.com/ http://textile.linkdata.cn http://www.oursh.com/ http://www.zrinfo.net/ http://djj.linkdata.cn http://lxj.linkdata.cn http://chuangye.linkdata.cn http://zhifu.linkdata.cn http://gzf.linkdata.cn http://fushi.linkdata.cn http://garment.linkdata.cn http://ddc.linkdata.cn http://health.linkdata.cn http://print2.linkdata.cn http://www.amusementtoys.com/en/ http://www.lanzhixiang.com/packdesign2/ http://www.lanzhixiang.com/marketingplan/ http://www.lanzhixiang.com/videsign/ http://www.lanzhixiang.com/logodesign/ http://wscl.linkdata.cn http://medical.linkdata.cn http://medequip.linkdata.cn http://medicalmachine.linkdata.cn http://www.imc.com.cn/0601.htm”>http://www.imc.com.cn/0601.htm http://www.imc.com.cn/yxzx.htm”>http://www.imc.com.cn/yxzx.htm http://www.liri-tents.com/ http://www.cndstv.com/ http://bump.linkdata.cn http://www.chinaruiqi.cn http://www.chinastv.net/ http://www.shttbc.com/ http://dz.win118.com/ http://www.sh8901.com/ http://sb.linkdata.cn http://sy.linkdata.cn http://qqad.linkdata.cn http://pmdesign.linkdata.cn http://qyxx.linkdata.cn http://realestate.linkdata.cn http://realestateplan.linkdata.cn http://adphoto.linkdata.cn http://adidea.linkdata.cn http://hainantrip.linkdata.cn http://report.linkdata.cn http://www.bjdfrw.com/ http://www.jx-dirui.com/c_products.htm http://www.jobstar.cn/ http://marketresearch3.linkdata.cn http://marketresearch2.linkdata.cn http://marketresearch.linkdata.cn http://research.linkdata.cn http://hklhc.linkdata.cn http://csb.linkdata.cn http://shinstallskytv.linkdata.cn http://installskytv.linkdata.cn http://shskytvinstall.linkdata.cn http://skytvinstall.linkdata.cn http://shskytv.linkdata.cn http://skytv3.linkdata.cn http://gift.linkdata.cn http://flower.linkdata.cn http://lhc9.linkdata.cn http://lhc8.linkdata.cn http://lhc7.linkdata.cn http://lhc6.linkdata.cn http://lhc5.linkdata.cn http://lhc4.linkdata.cn http://lhc3.linkdata.cn http://lhc2.linkdata.cn http://mold2.linkdata.cn http://lighting.linkdata.cn http://biaopai.linkdata.cn http://digitalcamera.linkdata.cn http://football.linkdata.cn http://xudianchi.linkdata.cn http://companyreg.linkdata.cn http://regcompany.linkdata.cn http://huangshan.linkdata.cn http://eye.linkdata.cn http://ad.linkdata.cn http://logodesign.linkdata.cn“>http://logodesign.linkdata.cn http://packdesign2.linkdata.cn“>http://packdesign2.linkdata.cn http://packdesign2.linkdata.cn“>http://packdesign2.linkdata.cn http://system.linkdata.cn http://fdmc.linkdata.cn http://instrument.linkdata.cn http://poker.linkdata.cn http://zhouyi.linkdata.cn http://name.linkdata.cn http://fengshui.linkdata.cn http://dryer.linkdata.cn http://ycjk.linkdata.cn http://video.linkdata.cn http://jszl.linkdata.cn http://jzzs.linkdata.cn http://office-furniture.linkdata.cn http://wuliu.linkdata.cn http://huoyun.linkdata.cn http://semi.linkdata.cn http://trademark.linkdata.cn http://outdoor-ad.linkdata.cn http://jinshi.linkdata.cn http://outdoorsports.linkdata.cn http://fuke.linkdata.cn http://ganyan.linkdata.cn http://ganbing.linkdata.cn http://hopeonline.linkdata.cn http://tuozhanex4.linkdata.cn http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn http://www.aptengfei.com/ http://www.diran.com.cn/ http://washmachine.linkdata.cn http://skytv2.linkdata.cn http://adcompany2.linkdata.cn http://logodesign.linkdata.cn“>http://logodesign.linkdata.cn http://videsign.linkdata.cn http://marketingplan.linkdata.cn http://slfs.linkdata.cn http://fan2.linkdata.cn http://www.h345.com/ http://www.sifanggl.com/ http://film.linkdata.cn http://cqwg2.linkdata.cn http://netgame.linkdata.cn http://www.huataiinfo.com/ http://adplan.linkdata.cn http://machinery.linkdata.cn http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn“>http://tuozhanex3.linkdata.cn http://adcompany.linkdata.cn http://cqwg.linkdata.cn http://managebook.linkdata.cn http://tuozhanex2.linkdata.cn http://job.linkdata.cn http://hire.linkdata.cn http://lube.linkdata.cn http://electrics.linkdata.cn http://heat.linkdata.cn http://wire.linkdata.cn http://battery.linkdata.cn http://gxsb.linkdata.cn http://adhesive.linkdata.cn http://spring.linkdata.cn http://clean.linkdata.cn http://officefurniture.linkdata.cn http://furniture.linkdata.cn http://www.minglu.org http://www.kghbjx.com/page12.htm http://www.iloveled.com http://www.hizjj.com/ http://www.pinganbanchang.com/company.htm http://www.shixinhy.com http://boiler.linkdata.cn“>http://boiler.linkdata.cn http://coaxial-cable.linkdata.cn http://yxwg.linkdata.cn http://legendserver.linkdata.cn http://toukui.linkdata.cn http://anquanmao.linkdata.cn http://mold.linkdata.cn http://gamewg.linkdata.cn http://wg.linkdata.cn http://www.amorphous.com.cn/ http://www.decocad.com/ http://www.amorphous.com.cn/en/aboutus/index.asp http://datarecover.linkdata.cn http://www.imc.com.cn/ad/”>http://www.imc.com.cn/ad/ http://byby.linkdata.cn http://huangshantrip.linkdata.cn http://www.shanbenbm.com/ http://huojia2.linkdata.cn http://cryp.linkdata.cn http://fan.linkdata.cn http://reducer.linkdata.cn http://exhibition.linkdata.cn http://print.linkdata.cn http://pack.linkdata.cn http://friend.linkdata.cn http://plasticpallet.linkdata.cn http://addesign2.linkdata.cn http://www.imc.com.cn/0304.htm”>http://www.imc.com.cn/0304.htm http://www.imc.com.cn/0303.htm”>http://www.imc.com.cn/0303.htm http://www.imc.com.cn/0306.htm”>http://www.imc.com.cn/0306.htm http://www.imc.com.cn/0402.htm”>http://www.imc.com.cn/0402.htm http://www.imc.com.cn/0309.htm”>http://www.imc.com.cn/0309.htm http://www.imc.com.cn/0305.htm”>http://www.imc.com.cn/0305.htm http://www.imc.com.cn/0310.htm”>http://www.imc.com.cn/0310.htm http://www.51mai.com/ http://www.qlx.org.cn/ http://toolmachine.linkdata.cn http://projector.linkdata.cn http://menjin.linkdata.cn http://www.zfwl.com http://www.zfwl.com/introduce.htm http://www.zfwl.com/products.htm http://www.zfwl.com/g1/g1.htm http://www.zfwl.com/dcq/dcq.htm http://www.zfwl.com/consult.htm http://planeticket.linkdata.cn http://plasticmachine.linkdata.cn http://ganxi.linkdata.cn http://crm.linkdata.cn http://cancer.linkdata.cn http://tumor.linkdata.cn http://laser.linkdata.cn“>http://laser.linkdata.cn http://www.paperease.com http://www.subin.cn http://www.booyee.com.cn http://www.net513.net http://beauty.linkdata.cn http://zkb.linkdata.cn http://cosmetic.linkdata.cn http://boiler.linkdata.cn“>http://boiler.linkdata.cn http://www.posh-design.com http://sxt588.vip.sina.com http://www.lt181.com http://www.wxconson.com/cp_2.htm http://www.wxconson.com/cp_1.htm http://www.xungeng.com/about.htm http://www.9966333.com http://www.nbpv.com/ryzs.htm”>http://www.nbpv.com/ryzs.htm http://www.yongbei.com http://www.ghystone.com http://www.ghystone.com/stone/carving/fountain/fountain.htm http://www.ghystone.com/stone/carving/column/column.htm http://www.ghystone.com/stone/tile/slate/slate-roofing.htm http://www.ghystone.com/stone/tile/granite/granite.htm http://www.ghystone.com/stone/carving/carved.htm http://www.ghystone.com/stone/tile/tile.htm http://www.ghystone.com/stone/carving/sink/sink.htm http://www.ghystone.com/stone/carving/fireplace/fireplace.htm http://liansuo.linkdata.cn http://jiameng.linkdata.cn http://zhaoshang.linkdata.cn http://laser.linkdata.cn“>http://laser.linkdata.cn http://NKK.linkdata.cn http://addesign.linkdata.cn http://carving.linkdata.cn http://power.linkdata.cn http://wirecable.linkdata.cn http://cable.linkdata.cn http://floor.linkdata.cn http://www.imc.com.cn/home.htm”>http://www.imc.com.cn/home.htm http://bbs.imc.com.cn/index.html”>http://bbs.imc.com.cn/index.html http://www.imc.com.cn/hyzx.htm”>http://www.imc.com.cn/hyzx.htm http://www.imc.com.cn/0307.htm”>http://www.imc.com.cn/0307.htm http://www.kaiguandianqi.com http://www.imc.com.cn/ad/”>http://www.imc.com.cn/ad/ppt/ggch.htm http://www.imc.com.cn/imc_plan/index.htm”>http://www.imc.com.cn/imc_plan/index.htm http://www.xmymy.com http://www.imc.com.cn/0308.htm”>http://www.imc.com.cn/0308.htm http://www.lsxch.com http://cizhuan.linkdata.cn http://glasssteel.linkdata.cn http://packdesign.linkdata.cn http://www.91jobs.com/ http://insulation.linkdata.cn http://yapianji.linkdata.cn http://YPj.linkdata.cn http://www.mjpk.net http://zhuche.linkdata.cn http://skytv.linkdata.cn http://coating.linkdata.cn http://feed.linkdata.cn http://jzcl.linkdata.cn http://gangjiegou.linkdata.cn http://buxiugang.linkdata.cn http://tuozhanex.linkdata.cn http://www.u513.com http://dwq.linkdata.cn http://tuozhan.linkdata.cn http://vpn3.linkdata.cn http://vpn001.freewebpage.org http://vpn001.51.net http://www.mp3car.cn/product2.htm”>http://www.mp3car.cn/product2.htm http://www.mp3car.cn/product3.htm”>http://www.mp3car.cn/product3.htm http://aoneng.linkdata.cn http://www.develot.com.cn/productfiles/20001.htm

  7. 前列腺炎 says:

    友情链接: [http://qlxsos.51.net 前列腺炎][http://ppp001.51.net 前列腺炎][http://www.qlxsos.com/ 前列腺炎]

Skip to main content