The point is to make the code better, not to satisfy R# :)

The main benefit of this process is marking fields as readonly, finding
code paths with stupid behavior and moving simple aggregations to use LINQ.
I don't apply the LINQ syntax to a non-trivial operations, to make it
easier to keep track of the Java version.

My thoughts on the points you raised inline

On Thu, Aug 2, 2012 at 6:53 PM, Zachary Gramana <zgram...@gmail.com> wrote:

> I would like to pitch into this effort and put my ReSharper license to
> use. I pulled down trunk, and picked a yellow item at random, and started
> to dig in. I quickly generated more questions than answers, realized I
> needed to stop munging code and consult the wiki and list archives. After
> digging through both, I'm still not entirely certain about what the style
> guidelines are for 3.x onward.
>
> I also noted this[1] discussion regarding some other guidelines, but it
> didn't see if it made it beyond the proposal stage.
>
> [1]
> http://mail-archives.apache.org/mod_mbox/lucene-lucene-net-dev/201112.mbox/%3ccajtrbsrdbzkocwln6d6ywhzn2fno91mko1acrp-pflx62du...@mail.gmail.com%3E
>
> Here are some of the things Re# is catching that I'm unsure of:
>
> 1) Usage of "this" prefix when not required.
>
> this.blah = blah;  <- required this.
> this.aBlah = blah; <- optional this, which Re# doesn't like.
>
> I'm assuming consistency wins here, and 'this.' stays, but wanted to
> double check.
>

Doesn't really matter IMO. I just hit Alt-enter when I have it in focus,
otherwise I ignore that.


>
> 2) Using different conventions for fields and parameters\local vars.
>
> blah vs. _blah
>
> Combined with 1, Re# wants (and I'm personally accustomed to):
>
> _blah = blah;
>
> However, that seems to violate the adopted style.
>

I think we should stick to the Java naming conventions in the private parts
(minus the function casings) as much as possible. Main reason is the
ability to apply patches from Java Lucene and support future ports more
easily. This is why I kept variable names untouched.


>
> 3) Full qualification of type names.
>
> Re # wants to remove redundant namespace qualifiers. Leave them or remove
> them?
>

Same as Alt-Enter argument as above...


>
> 4) Removing unreferenced classes.
>
> Should I remove non-public unreferenced classes? The ones I've come across
> so far are private.
>

It's .NET, not C++, but I still usually remove them, not really sure why
tho...


>
> 5) var vs. explicit
>
> I know this has been brought up before, but not sure of the final
> disposition. FWIW, I prefer var.
>
>
> There are some non-Re# issues I came across as well that look like
> artifacts of code generation:
>

I move to var because it *might* help in the future when the API changes,
and it doesn't really affect anything now


>
> 6) Weird param names.
>
> Param1 vs. directory
>
> I assume it's okay to replace 'Param1' with something a descriptive name
> like 'directory'.
>

Yes. Also var names like out_Renamed to @out. This one is important.


>
> 7) Field names that follow local variable naming conventions.
>
> Lots of issues related to private vars with names like i, j, k, etc. It
> feels like the right thing to do is to change the scope so that they go
> back to being local vars instead of fields. However, this requires a much
> more significant refactoring, and I didn't want to assume it was okay to do
> that.
>

See above, I don't think we should touch those.


>
> If these questions have already been answered elsewhere and I missed the
> documentation/FAQ/developer guide, then I apologize and would appreciate
> the links. Alternatively, if someone has a Re# rule config that they are
> willing to post somewhere, I would be glad to use it.
>
> - Zack
>
>
> On Jul 27, 2012, at 12:00 PM, Itamar Syn-Hershko wrote:
>
> > The cleanup consists mainly of going file by file with ReSharper and
> trying
> > to get them as green as possible. Making a lot of fields readonly,
> removing
> > unused vars and stuff like that. There are still loads of files left.
> >
> > I was also hoping to get to updating the spatial module with some recent
> > updates, and to also support polygon searches. But that may take a bit
> more
> > time, so it's really up to you guys (or we can open a vote for it).
>
>
>

Reply via email to