castor

Refactor javasource package to resolve 'Design by extension' complaints of checkstyle

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Trivial Trivial
  • Resolution: Fixed
  • Affects Version/s: 1.1 M2
  • Fix Version/s: 1.1 M3
  • Component/s: XML code generator
  • Labels:
    None
  • Number of attachments :
    2

Description

Summary says it all.

  1. Text File
    patch-C1790-20070102.txt
    01/Jan/07 7:37 PM
    394 kB
    Ralf Joachim
  2. Text File
    patch-C1790-20070102-01.txt
    02/Jan/07 4:16 AM
    18 kB
    Ralf Joachim

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Edward Kuns added a comment -

I much prefer

if (jType.isPrimitive()) {

over

if (jType instanceof JPrimitiveType) {

because the former communicates intent and the second communicates only membership. That is, with the second, we cannot transparently have another type return isPrimitive() true. Thus, changes to the concept of isPrimitive are no longer encapsulated.

I don't like the idea of making this sort of change just to make a checkstyle warning go away.

Show
Edward Kuns added a comment - I much prefer if (jType.isPrimitive()) { over if (jType instanceof JPrimitiveType) { because the former communicates intent and the second communicates only membership. That is, with the second, we cannot transparently have another type return isPrimitive() true. Thus, changes to the concept of isPrimitive are no longer encapsulated. I don't like the idea of making this sort of change just to make a checkstyle warning go away.
Hide
Permalink
Ralf Joachim added a comment -

After I had finished the patch I thought about the same. Will reopen the issue and introduce/use 2 methods at JType:

boolean isPrimitive()

{ return (this instanceof JPrimitiveType); }

boolean isArray()

{ return (this instanceof JArrayType); }

This should not introduce new warnings while it encapsulates implementation. Having said that the extraction of JPrimitiveType may also allow us to get rid of some conditional statements at futur refactorings.

Show
Ralf Joachim added a comment - After I had finished the patch I thought about the same. Will reopen the issue and introduce/use 2 methods at JType: boolean isPrimitive() { return (this instanceof JPrimitiveType); } boolean isArray() { return (this instanceof JArrayType); } This should not introduce new warnings while it encapsulates implementation. Having said that the extraction of JPrimitiveType may also allow us to get rid of some conditional statements at futur refactorings.
Hide
Permalink
Ralf Joachim added a comment -

As commented above.

Show
Ralf Joachim added a comment - As commented above.
Hide
Permalink
Ralf Joachim added a comment -

Additional changes as explained above.

Show
Ralf Joachim added a comment - Additional changes as explained above.
Hide
Permalink
Edward Kuns added a comment -

Yes, introducitng JPrimitiveType is definitely a nice thing.

Show
Edward Kuns added a comment - Yes, introducitng JPrimitiveType is definitely a nice thing.

People

  • Assignee:
    Ralf Joachim
    Reporter:
    Ralf Joachim
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: