Yup, no problem i'll try to do it while it's still spring :) Are smaller patches better or is just a large chunk better?
regards Nino 2010/3/25 Igor Vaynberg <[email protected]>: > feel free to do this, but for trunk only. i would rather not tweak > working code for the sake of making it "cleaner". > > also for this specific example there is Streams.close() that does this > already... > > -igor > > On Thu, Mar 25, 2010 at 5:26 AM, nino martinez wael > <[email protected]> wrote: >> Well I do think there are some places that where code are somewhat >> unreadable or hard to read. So it was just to make the code more >> readable.. And making maintenance time go down. So some of it would be >> following up against rule violations, but some of it would also be >> tidying up code like this (from fileupload.java on 1.4.x branch), and >> it can be further reduced below are just example: >> >> [original] >> public final void closeStreams() >> { >> if (inputStreamsToClose != null) >> { >> for (Iterator<InputStream> inputStreamsIterator = >> inputStreamsToClose.iterator(); inputStreamsIterator.hasNext();) >> { >> InputStream inputStream = >> inputStreamsIterator.next(); >> >> try >> { >> inputStream.close(); >> } >> catch (IOException e) >> { >> // We don't care aobut the exceptions >> thrown here. >> } >> } >> >> // Reset the list >> inputStreamsToClose = null; >> } >> } >> [modified] >> public final void closeStreams() >> { >> if (inputStreamsToClose != null) >> { >> for (Iterator<InputStream> inputStreamsIterator = >> inputStreamsToClose.iterator(); inputStreamsIterator.hasNext();) >> { >> closeInputStream(inputStreamsIterator.next()); >> } >> >> // Reset the list >> inputStreamsToClose = null; >> } >> } >> >> private void closeInputStream(InputStream inputStream){ >> try >> { >> inputStream.close(); >> } >> catch (IOException e) >> { >> // We don't care aobut the exceptions >> thrown here. >> } >> >> } >> >> It's up to you guys, it's defiantly not an easy task. Im just >> following the example from Robert C Martin >> (http://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882) >> ... In my daily work I've found it to be a very good practice. >> >> >> >> 2010/3/23 Igor Vaynberg <[email protected]>: >>> lets not start tweaking code for tweaking's sake. >>> >>> -igor >>> >>> On Tue, Mar 23, 2010 at 11:57 AM, nino martinez wael >>> <[email protected]> wrote: >>>> Yeah I know, one has to have a sense of code regarding some of those >>>> violations. But are very good to have as an indicator, at work I've >>>> removed some of the rules from the rule set as they do not fit our >>>> programming style. We value code readability very high, although it's >>>> a constant battle between developers. >>>> >>>> Regarding reading code is a completely different matter. When I see >>>> something like "!foo", I just read it "not foo". You can miss the >>>> exclamation, but you can also miss one equal sign and have a fatal >>>> problem. >>>> >>>> One of the most annoying ones are if you create something serializable >>>> you also have to declare an id. Another one are that eclipse by >>>> default generates methods that are pulled to an interface abstract, >>>> but one of the rules says it's duplicate signature since it's an >>>> interface :) So theres a secret battle between eclipse and the >>>> ruleset.. >>>> >>>> But most of the are good. >>>> >>>> regards Nino >>>> >>>> 2010/3/23 Jeremy Thomerson <[email protected]>: >>>>> I would reject patchs to fix some of those. Some of those so-called >>>>> "violations" are just their coding style not being the same as ours. >>>>> >>>>> For instance, they say there are 218 "violations" where we have 'if (foo >>>>> == >>>>> false)' - which they say should be simplified, I'm assuming to be 'if >>>>> (!foo)'. Personally, I write mine as "foo == false" because it is much >>>>> harder to miss that than it is to miss "!" as you're reading through the >>>>> code. >>>>> >>>>> Another example: "empty method in abstract class should be abstract". No, >>>>> it shouldn't. It's a method designed to be overridden for additional >>>>> functionality if you so desire. >>>>> >>>>> There might be some that are worth fixing. But as I mention, there are >>>>> some >>>>> that are better left alone. >>>>> >>>>> -- >>>>> Jeremy Thomerson >>>>> http://www.wickettraining.com >>>>> >>>>> >>>>> >>>>> On Tue, Mar 23, 2010 at 6:39 AM, nino martinez wael < >>>>> [email protected]> wrote: >>>>> >>>>>> Hi I wondered >>>>>> >>>>>> if it would be interesting if I started to make wicket more in >>>>>> compliance with the rules defined here: >>>>>> http://nemo.sonarsource.org/drilldown/violations/44196?priority=MAJOR >>>>>> ? >>>>>> >>>>>> I'd of course start by submitting patches.. >>>>>> >>>>>> So are it interesting? >>>>>> >>>>>> regards Nino >>>>>> >>>>> >>>> >>> >> >
