Bug 28068 - [quick fix] Greate getter for "Field is not visible"
Summary: [quick fix] Greate getter for "Field is not visible"
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 2.1   Edit
Hardware: All All
: P3 enhancement with 1 vote (vote)
Target Milestone: 3.3 RC2   Edit
Assignee: Karsten Becker CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
: 26824 65597 75870 78796 (view as bug list)
Depends on:
Blocks:
 
Reported: 2002-12-10 23:56 EST by Konstantin Scheglov CLA
Modified: 2007-06-06 12:25 EDT (History)
5 users (show)

See Also:
martinae: review+
benno.baumgartner: review+


Attachments
Change to getter/setter if available (20.48 KB, patch)
2007-03-28 11:26 EDT, Karsten Becker CLA
no flags Details | Diff
Invoke SelfEncapsulateField if not Getter/Setter (58.87 KB, patch)
2007-04-03 10:21 EDT, Karsten Becker CLA
no flags Details | Diff
Patch after Review (72.88 KB, patch)
2007-04-05 10:43 EDT, Karsten Becker CLA
no flags Details | Diff
Additional Tests (28.91 KB, patch)
2007-05-04 11:22 EDT, Karsten Becker CLA
no flags Details | Diff
Addressed open issues (29.79 KB, patch)
2007-05-24 10:03 EDT, Karsten Becker CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Scheglov CLA 2002-12-10 23:56:03 EST
I have two classes:
A - with private field and getter for it.
public
class A {
  private int m_Field;
  public int getField() {
    return m_Field;
  }
}
And B, which attepts to use field from A (accidently, due "Move" refactoring)
class B {
  private A a;
  public int foo() {
    return a.m_Field * 2;
  }
}
  Now there is only one possible fix for "a.m_Field" - change visibility. Would be great to have also item for "Use getter" or someting like this.
Comment 1 Martin Aeschlimann CLA 2003-03-31 11:15:20 EST
*** Bug 26824 has been marked as a duplicate of this bug. ***
Comment 2 Martin Aeschlimann CLA 2003-04-02 10:12:34 EST
*** Bug 26824 has been marked as a duplicate of this bug. ***
Comment 3 Martin Aeschlimann CLA 2003-04-02 10:13:23 EST
bug 26824 suggests to offer to create a getter when not existing
Comment 4 Alex Blewitt CLA 2003-04-02 10:17:27 EST
Dirk also commented that a 'create getter' should have higher priority 
than 'change visibility to public'
Comment 5 Martin Aeschlimann CLA 2004-05-12 14:31:57 EDT
not for 3.0
Comment 6 Martin Aeschlimann CLA 2004-10-08 04:42:54 EDT
reopened for 3.1
Comment 7 Martin Aeschlimann CLA 2004-10-08 04:43:09 EDT
*** Bug 75870 has been marked as a duplicate of this bug. ***
Comment 8 Thorbjørn Ravn Andersen CLA 2004-10-08 09:13:52 EDT
I agree that the "create getter" should have a higher priority than "make
visible". It would be fine with me if the create-getter action would invoke the
usual Create Getter/Setter dialogue, with only the getter checked.
Comment 9 Martin Aeschlimann CLA 2007-03-22 06:28:57 EDT
*** Bug 65597 has been marked as a duplicate of this bug. ***
Comment 10 Martin Aeschlimann CLA 2007-03-22 06:29:34 EDT
*** Bug 78796 has been marked as a duplicate of this bug. ***
Comment 11 Karsten Becker CLA 2007-03-28 11:26:59 EDT
Created attachment 62241 [details]
Change to getter/setter if available

This patch does not offer to create a getter/setter yet. It simply offers to change to an available one. The detection of getters/setters for fields with prefixes does not work yet because of https://bugs.eclipse.org/bugs/show_bug.cgi?id=179506
Comment 12 Karsten Becker CLA 2007-04-03 10:21:28 EDT
Created attachment 62780 [details]
Invoke SelfEncapsulateField if not Getter/Setter

The SEF dialog has been slightly improved and ignores problems that are likely to be solved by this refactoring.
Comment 13 Philipe Mulet CLA 2007-04-03 13:12:08 EDT
+1 for 3.3M7
Comment 14 Karsten Becker CLA 2007-04-05 10:43:51 EDT
Created attachment 63039 [details]
Patch after Review

Checks for static modifier match of field and method. UI disables not applicable fields.
Comment 15 Martin Aeschlimann CLA 2007-04-13 09:45:44 EDT
patch released > 20070413

to be done:
- include quick fix for unqualified again. I commented it out as existing tests need to be adapted
- avoid testing the order of tests
- think of adding 'Encapsulate field' as quick assist.
Comment 16 Karsten Becker CLA 2007-05-04 11:22:34 EDT
Created attachment 65931 [details]
Additional Tests

Unqualified Tests in LocalCorrections updated
SEF Test in ModifierCorrectionsTest
Comment 17 Martin Aeschlimann CLA 2007-05-24 07:05:08 EDT
patch is good (not minimal, but the formatting changes are good)
Comment 18 Martin Aeschlimann CLA 2007-05-24 07:05:36 EDT
Benno, can you do the second review?
Comment 19 Benno Baumgartner CLA 2007-05-24 08:58:57 EDT
- Do not even THINK about catching java.lang.Exception;-)
- Do not print to console (e.printStackTrace())
- Fields in ProposalParameter must have f prefix
- Use getters for private fields
- Not sure about this, but I usually see the following pattern in our code:
if (Display.getCurrent() != null) {
	perform(...);
} else {
	Display.getDefault().syncExec(new Runnable() {
		public void run() {
			perform(...);
		}
	});
}
Ask Martin...

Comment 20 Karsten Becker CLA 2007-05-24 10:03:48 EDT
Created attachment 68572 [details]
Addressed open issues

ProposalParameter fields do not start with f because they are parameters as discussed with martin. That is why they also do not have getters.
Comment 21 Benno Baumgartner CLA 2007-05-24 10:43:54 EDT
fixed > I20070524-0010

Changed:
- Correct formatting
- Made fields 'public'
- Using Display.getDefault()
Comment 22 Benno Baumgartner CLA 2007-05-29 04:34:10 EDT
verified in I20070527-0010

Filed Bug 189606