The advantage of using "final" in the parameter list is that you aren't
overriding the value that
was passed in -- no matter where you are in the method, the name of the
arguments still refer to the
objects passed as arguments. At least, that's true if you also outlaw
shadowing (which is another
thing I usually do).
The problem is that people mistakenly think this should work:
void fooize(String notfoo) {
notfoo = "foo"
}
String bar = "bar"
fooize(bar)
assertEquals "foo", bar
That will be a compile error if you make "notfoo" final. In general, slapping
"final" in the
parameter list clears up the whole pass-by-reference/pass-by-value quandary.
~~ Robert.
kirk wrote:
> Reinier Zwitserloot wrote:
>> In regards to Robert Fischer's 'all parameters must be final': That's
>> exactly the kind of language war that should not be the focus of code
>> reviews. Broach the subject with the team, and if there's clear
>> concensus that one way is the right way, then, sure, from that moment
>> onwards, enforce it in code reviews, but if there's no consensus, then
>> don't bother.
>>
> I like all code styles that match mine!
>
> really, this is what code formatters are for...
>
> I fail to see the benefit of using final in a parameter list.
>
> Regards,
> Kirk
>
>> On Feb 12, 3:45 pm, jvb <[email protected]> wrote:
>>
>>> Thanks for the information.
>>>
>>> In what you describe (and this seems to be what most of the tools
>>> assume as well) the review comes before the commit (at least before
>>> the "real" commit on the trunk or whatever). Currently we all freely
>>> commit without first checking with somebody if we are allowed to or
>>> whatever. This also means that we currently usually don't branch for
>>> that reason, we commit directly on the main trunk. We do make branches
>>> of course, but for other reasons (to support older versions etc).
>>>
>>> I'm a bit afraid that this will have a huge impact on the way we
>>> currently work. Won't it slow us down, certainly because it is
>>> probably not necessary to have everything code reviewed? Could a code
>>> review process which reviews code after the commit (but before
>>> releasing to for example the testing team) also work? Or would this
>>> lead to a process which gets too informal?
>>>
>>> Jan
>>>
>>> On Feb 12, 11:56 am, Reinier Zwitserloot <[email protected]> wrote:
>>>
>>>
>>>> In my opinion, code reviews work best when you keep it informal and
>>>> don't have a checklist of specific issues. Just look at the code, try
>>>> to understand what it does, and then nitpick at it. If you think it
>>>> could be expressed easier, then either make a suggestion or just make
>>>> a fix. Whatever you see that seems like it could be better - say it.
>>>> Don't get into a style war, but there's a difference between 'I think
>>>> that open bracket would look better on the same line as the if
>>>> block' (don't mention that unless the team decided to do it one way),
>>>> and "you're doing a .containsKey() check and then .get()ing the key.
>>>> Don't do that - just get the key and compare to null afterwards,
>>>> because that's much easier to fit into a multithreading model." (do
>>>> mention that, especially if you know you're already using this map in
>>>> a multi-threaded environment, or that it might be likely, and even if
>>>> you don't, there are going to be situations where it is relevant, and
>>>> when faced with a choice, doing it the same way every time is better).
>>>>
>>>> Code reviews should be a two-way street. If you think something
>>>> doesn't fit in the architectural plans, then open a discussion, and if
>>>> the two of you can't figure it out, take it to the group (there's a
>>>> lot of advantages to working with a small group, so use it!) If you as
>>>> a reviewer make changes instead of suggestions (which should
>>>> definitely be allowed), always send it back upstream for a counter-
>>>> review by the original programmer.
>>>>
>>>> Use a code review system and do code reviews personally, not in a
>>>> group. Basically, anybody that has a commit, toss it in the 'to-be-
>>>> reviewed' branch so the reviewer can pull it, or just push a git
>>>> branch over to the reviewer, and the reviewer then gets back to the
>>>> original programmer with either:
>>>> - a signoff,
>>>> - a signoff with conditions (minor nits that you trust the programmer
>>>> can fix without needing another review)
>>>> - a date to meet up and discuss issues face to face
>>>> - a realization that this is particularly crucial to the entire
>>>> group. This may indeed warrant sitting everyone in front of a
>>>> projector. But should be a very rare event.
>>>>
>>>> There are a bunch of tools out there. I know google takes their own
>>>> code review process rather seriously, but I'm not sure if the tool
>>>> they use (isn't it called mondrian, written in python by Guido van
>>>> Rossum?) is available for use outside of google. A DVCS can help a lot
>>>> by allowing people to use 1 branch per thingie-to-be-code-reviewed,
>>>> and to allow people to push and pull these branches around without
>>>> needing a central server. I strongly recommend git or hg (and,
>>>> eventhough hg has some sway in java land, git does seem to have the
>>>> momentum).
>>>>
>>>> And, whatever you do, make sure everyone agrees that life is better
>>>> with the code reviews compared to without them. If its a management
>>>> dictatorial decision, then programmers WILL find a way around it, -or-
>>>> if they don't like each other, are going to use it as a forum for
>>>> asinine nitpicking instead of constructive criticism.
>>>>
>>>> On Feb 12, 11:01 am, jvb <[email protected]> wrote:
>>>>
>>>>> In our company, we are thinking about introducing some formalized form
>>>>> of peer code review. We (the Java guys) are a small team of 5
>>>>> developers only, and in practice we already discuss each others code
>>>>> all the time (informally). There are of course some highly specialized
>>>>> parts of the codebase that are almost exclusively known by one or two
>>>>> persons only, certainly because we don't work on a single application
>>>>> but more a whole suite of applications (used to manage satellite earth
>>>>> stations).
>>>>>
>>>>> The person who wants to introduce this idea of a more formalized peer
>>>>> code review comes from a different enviroment (mainframe stuff) where
>>>>> the write/compile/test cycle was much slower. In his environment it
>>>>> was obvious that code review (even before compiling if I understand
>>>>> correctly) is very usefull to find bugs which would otherwize slow
>>>>> down the already slow write/compile/test cycle even more.
>>>>>
>>>>> Now I'm thinking how we can translate this to our java development
>>>>> environment. We are obviously not going to review code to find compile
>>>>> errors ;-)
>>>>>
>>>>> - What kind of errors should we focus on when doing code review
>>>>> (style? concurrency issues? use of patterns? whether it fits in the
>>>>> overall architecture? whether the code actually implements the
>>>>> requirements? ...)
>>>>> - How do we best organize these peer reviews such that they will
>>>>> provide a real benefit, knowing that our day to day way of working
>>>>> already implies working with each others code a lot?
>>>>> - Should we be focussing on the parts of the code that are less known
>>>>> by everybody?
>>>>> - Should we be sitting together around a screen/projector, or should
>>>>> we simply assign parts of the codebase to be reviewed by certain
>>>>> developers on their own?
>>>>> - Should we be looking at tool support?
>>>>> - ...
>>>>>
>>>>> Does anybody has any experience and/or usefull information to share?
>>>>>
>>>>> many thanks in advance,
>>>>> Jan
>>>>>
>>
>
>
> >
>
--
~~ Robert Fischer.
Grails Trainining http://GroovyMag.com/training
Smokejumper Consulting http://SmokejumperIT.com
Enfranchised Mind Blog http://EnfranchisedMind.com/blog
Check out my book, "Grails Persistence with GORM and GSQL"!
http://www.smokejumperit.com/redirect.html
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "The
Java Posse" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/javaposse?hl=en
-~----------~----~----~----~------~----~------~--~---
