A Record Class Type: Bad Style?

[George W. Cherry]

I want to return three named ints from a method. I've done
it (as shown below) with a class that has three public int
fields. A pal of mine said this is execrable style. Is she
right? What would be the elegant received style of doing
what I want to do? (An old Ada programmer wants to know.)

George (Old Ada Programmer)

public class CountTheCharacters {
public static class NumberOf {
public int vowels;
public int consonants;
public int others;
}

public static NumberOf countTheCharacters(String str) {
NumberOf numberOf = new NumberOf();
for (int j = 0; j < str.length(); j = j + 1) {
char ch = str.charAt(j);
if ( Character.isLetter(ch) ) {
switch(ch) {
case 'a': case 'e': case 'i': case 'o': case 'u':
case 'A': case 'E': case 'I': case 'O': case 'U':
numberOf.vowels++;
break; //end cases for vowels

default:
numberOf.consonants++;
//end ch is a letter but not a vowel
}//end switch statement
}
else { //ch is not a letter
numberOf.others++;
}
}//end for loop
return numberOf;
}//end countTheCharacters method
}//end class

 

[Hal Rosser]

You COULD just return an int array - rather than expose your public
variables.

 

[xarax]

I want to return three named ints from a method. I've done
it (as shown below) with a class that has three public int
fields. A pal of mine said this is execrable style. Is she
right? What would be the elegant received style of doing
what I want to do? (An old Ada programmer wants to know.)

George (Old Ada Programmer)

public class CountTheCharacters {
public static class NumberOf {
public int vowels;
public int consonants;
public int others;
}

public static NumberOf countTheCharacters(String str) {
NumberOf numberOf = new NumberOf();
for (int j = 0; j < str.length(); j = j + 1) {
char ch = str.charAt(j);
if ( Character.isLetter(ch) ) {
switch(ch) {
case 'a': case 'e': case 'i': case 'o': case 'u':
case 'A': case 'E': case 'I': case 'O': case 'U':
numberOf.vowels++;
break; //end cases for vowels

default:
numberOf.consonants++;
//end ch is a letter but not a vowel
}//end switch statement
}
else { //ch is not a letter
numberOf.others++;
}
}//end for loop
return numberOf;
}//end countTheCharacters method
}//end class

It is alright as is, but I would rather see the NumberOf
object passed as a parameter to countTheCharacters() instead
of creating a new object for every call. The caller can
then decide when to create a new object. Just be sure to
reset the fields to zero before the for() loop. That will
also allow the caller to use an extension (subclass) of
NumberOf for whatever purpose.

Two cents worth. Your mileage may vary.

 

[Chris Smith]

It is alright as is, but I would rather see the NumberOf
object passed as a parameter to countTheCharacters() instead
of creating a new object for every call. The caller can
then decide when to create a new object. Just be sure to
reset the fields to zero before the for() loop. That will
also allow the caller to use an extension (subclass) of
NumberOf for whatever purpose.

Two cents worth. Your mileage may vary.

Indeed. Off-hand, I'd disagree with that suggestion. What you're
suggesting is an optimization, and one that's less clear, not guaranteed
to improve things, and in fact quite likely to hurt performance in some
situations. It should be implemented only if profiling indicates that
(1) there is in fact a performance problem, and (2) this solves it.

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Chris Smith]

I want to return three named ints from a method. I've done
it (as shown below) with a class that has three public int
fields. A pal of mine said this is execrable style. Is she
right? What would be the elegant received style of doing
what I want to do? (An old Ada programmer wants to know.)

Good question. What you've done isn't necessarily bad, but I'd change a
few things:

1. The class name. Good naming does wonders for the readability of
code. Your sort of gimmicky name "NumberOf" is a bit confusing when
used in the standard way as a noun phrase. I'd call the class
"CharacterCount" or something to that effect.

2. Replace public fields with methods, like getVowels() or
getVowelCount().

3. Instead of using a static method and a nested class, you might
consider placing the counting code inside the same class that
encapsulates the results -- either in a constructor or still in a static
method (in which latter case, it would be essentially be a factory
method).

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Chris Uppal]

3. Instead of using a static method and a nested class, you might
consider placing the counting code inside the same class that
encapsulates the results.

Agreed, and that's the way I'd do this.

I want to add:

You (the OP) may find, once you have properly sorted out the object
responsibilities, and created an object with the job of analysing character
data, that more of your code "wants" to live in that object. So consider what
you are doing with the data after you have collected it, is any of that
functionality something that should be part of CharacterCount's (or
CharacterAnalysis's) responsibility ?

OTOH, you could go completely the other way. As far as I can see, the
original NumberOf class existed /solely/ as an optimisation to avoid scanning
the string three times. Is that optimisation necessary ? If not then just
scan three times and be done with it. Of course, it may well be that you
prefer the code structure you get by following Chris's advice and using a
proper object, in which case the optimisation is just icing on the cake.

Incidentally, the way you categorise your characters seems very ASCII
specific -- it will break if you are treating full Unicode, or even just Java's
cut-down imitation of it. I don't know if that's just because you brewed up
the example simply /as/ an example, or it is representative of your real code,
but I thought I'd mention it just in case. Notice, by the way, that once you
have created an object whose job is to categorise characters, replacing it with
another object, which does the same job but is Unicode aware, should be very
easy.

-- chris

 

[George W. Cherry]

You COULD just return an int array - rather than expose your public
variables.

That would be execrable: the user would have to
use array indexes (0, 1, or 2) instead of suggestive
names for the variables. What is this "indecent
exposer" fear you have?

George

 

[George W. Cherry]

message


It is alright as is, but I would rather see the NumberOf
object passed as a parameter to countTheCharacters() instead
of creating a new object for every call.

Your suggestion is:
public static NumberOf void(String str, NumberOf numberOf)

My problem with this is that the numberOf object may
not have its fields set to zero, and so the method must
do that. Why not let the method create the new numberOf
object as I did? It seems to me that the method should be
the guy who creates the object. The user should call my
method this way:

NumberOf numberOf = countTheCharacters(str);

All of this works because Java guarantees that the int
fields of a newly created object are set to zero.

The caller can
then decide when to create a new object. Just be sure to
reset the fields to zero before the for() loop. That will
also allow the caller to use an extension (subclass) of
NumberOf for whatever purpose.

Two cents worth. Your mileage may vary.

Okay,

George

 

[Christian Hvid]

Personally I would have written three methods:

- countVowels
- countConsonants
- countNonAlphabetic (or something ...)

(You can make them call a general private method in your class).

It is common to use getters and setters in Java. Because you have finer
control over accessability, you can change the implementation (the way you
store your data) and you "program against an interface". So your "NumberOf"
class would look like:

public static class NumberOf {
private int vowels;
...
public int getNoVowels() { return vowels; } // no = number of
...
private NumberOf(int vowels, int consonants, int others) {
this.vowels = vowels;
...
}
}

(The class is "immutable" - its internals are only set through the
constructor and it is thus safe to pass the object around by reference.)

But that is only to demonstrate an irrelevant point.

I don't think it is a good a idea to introduce a NumberOf class. Simply
because it (the "NumberOf") is not an "object", not a thing ...

-- Christian
http://vredungmand.dk

 

[George W. Cherry]

Good question. What you've done isn't necessarily bad, but I'd change a
few things:

1. The class name. Good naming does wonders for the readability of
code. Your sort of gimmicky name "NumberOf" is a bit confusing when
used in the standard way as a noun phrase. I'd call the class
"CharacterCount" or something to that effect.

Agreed. The following would be better.

public static class CharacterCounts {
public int numberOfVowels;
public int numberOfConsonants;
public int numberOfOthers;
}

2. Replace public fields with methods, like getVowels() or
getVowelCount().

3. Instead of using a static method and a nested class, you might
consider placing the counting code inside the same class that
encapsulates the results -- either in a constructor or still in a static
method (in which latter case, it would be essentially be a factory
method).

Yes to 2. and 3.--IF I had a real problem where I needed
to compute three numbers which really should be returned
together. The example I made up for illustrating the issue
was a poor one I think, as I argue below.

George

 

[Hal Rosser]

That would be execrable: the user would have to
use array indexes (0, 1, or 2) instead of suggestive
names for the variables. What is this "indecent
exposer" fear you have?

George

well then - do it your way - its your app

 

[Alan Gutierrez]

I want to return three named ints from a method. I've done
it (as shown below) with a class that has three public int
fields. A pal of mine said this is execrable style. Is she
right? What would be the elegant received style of doing
what I want to do? (An old Ada programmer wants to know.)

George (Old Ada Programmer)

/**
* Counts vowels, consonants, and other characters. This class is
* immutable. Warning: this class only works with English, and only
* if you consistantly misspell words like cafe.
*
* @author George (Old Ada Programmer)
* @author Alan Gutierrez
*/
public class CharacterReport {

private final int vowels;
private final int consonants;
private final int others;

public CharacterReport(String str) {
int vowels = 0;
int consonants = 0;
int others = 0;

for (int j = 0; j < str.length(); j = j + 1) {
char ch = str.charAt(j);
if ( Character.isLetter(ch) ) {
switch(ch) {
case 'a': case 'e': case 'i': case 'o': case 'u':
case 'A': case 'E': case 'I': case 'O': case 'U':
vowels++;
break; //end cases for vowels

default:
consonants++;
//end ch is a letter but not a vowel
}
}
else { //ch is not a letter
others++;
}
}

this.vowels = vowels;
this.consonants = consonants;
this.others = others;
}

public int getVowelCount() {
return vowels;
}

public int getConsonantCount() {
return consonants;
}

public int getOtherCount() {
return others;
}
}


The above is how I would implement the class. First of,
CharacterReport is an proper object. It has a state, and it has
logic to create that state, all rolled into one. This replaces
the NumberOf / CountTheCharacters duo.

The resulting object is immutable, thus the final keyword can be
applied to the member variables.

I would not write three methods. Not because of performance, but
because logically, I can imagine that if you are counting
vowels, you are going to want to count consonants. If this is a
report you are generating, generate the report, and expose it's
properties.

Hope this helps.

Alan Gutierrez - (e-mail address removed)

 

[George W. Cherry]

/**
* Counts vowels, consonants, and other characters. This class is
* immutable. Warning: this class only works with English, and only
* if you consistantly misspell words like cafe.

: o ) What's that funny mark Frenchmen put above some e's?
(More below). George

*
* @author George (Old Ada Programmer)
* @author Alan Gutierrez
*/
public class CharacterReport {

private final int vowels;
private final int consonants;
private final int others;

public CharacterReport(String str) {
int vowels = 0;
int consonants = 0;
int others = 0;

for (int j = 0; j < str.length(); j = j + 1) {
char ch = str.charAt(j);
if ( Character.isLetter(ch) ) {
switch(ch) {
case 'a': case 'e': case 'i': case 'o': case 'u':
case 'A': case 'E': case 'I': case 'O': case 'U':
vowels++;
break; //end cases for vowels

default:
consonants++;
//end ch is a letter but not a vowel
}
}
else { //ch is not a letter
others++;
}
}

this.vowels = vowels;
this.consonants = consonants;
this.others = others;
}

public int getVowelCount() {
return vowels;
}

public int getConsonantCount() {
return consonants;
}

public int getOtherCount() {
return others;
}
}

The above is how I would implement the class. First of,
CharacterReport is an proper object. It has a state, and it has
logic to create that state, all rolled into one. This replaces
the NumberOf / CountTheCharacters duo.

The resulting object is immutable, thus the final keyword can be
applied to the member variables.

I would not write three methods. Not because of performance, but
because logically, I can imagine that if you are counting
vowels, you are going to want to count consonants. If this is a
report you are generating, generate the report, and expose it's
properties.

Yes, that's one nice way to do it, and it puts everything
to do with this little problem into one little class. In fact,
your solution should have some sort of pattern name.

BUT (and I don't want to sound ungrateful) there's something
a little unnatural about it to my procedural (methodic?) sense.
I just want a little method to which I pass an argument and
which returns a result. Passing an argument to a "method"
via a constructor and making the constructor a kind of de facto
method seems--to me--a little bit of a misuse of the idea of
constructors. I think that your solution would surprise a lot of
java users. So, I submit the following as perhaps a more
"natural" solution.
_______________________________________________________
public class CharacterAnalysis {
public static class CharacterCounts {
private int vowels;
private int consonants;
private int others;
public int getNumberOfVowels() { return vowels; }
public int getNumberOfConsonants() { return consonants; }
public int getNumberOfOthers() { return others; }
}

public static CharacterCounts getCounts(String str) {
CharacterCounts count = new CharacterCounts();
for (int j = 0; j < str.length(); j = j + 1) {
char ch = str.charAt(j);
if ( Character.isLetter(ch) ) {
switch(ch) {
case 'a': case 'e': case 'i': case 'o': case 'u':
case 'A': case 'E': case 'I': case 'O': case 'U':
count.vowels++;
break; //end cases for vowels

default:
count.consonants++;
//end ch is a letter but not a vowel
}//end switch statement
}
else { //ch is not a letter
count.others++;
}
}//end for loop
return count;
}//end countTheCharacters method

/* The following method tests getCounts. */
public static void main(String[] args) {
StringBuffer buffer = new StringBuffer();
for (int j = 0; j < args.length; j = j + 1) {
buffer.append( (j == 0 ? "" : " ") + args[j] );
}
String str = buffer.toString();
CharacterCounts count = getCounts(str);
System.out.println(str);
System.out.println( "Number of vowels = " +
count.getNumberOfVowels() );
System.out.println( "Number of consonants = " +
count.getNumberOfConsonants() );
System.out.println( "Number of others = " +
count.getNumberOfOthers() );
}//end main()
}//end class
______________________________________________________________________

George

 

[George W. Cherry]

well then - do it your way - its your app

I didn't intend to appear to diss your suggestion.
Indeed, I think it has merit. My problem with returning
an int array is that the user does not know which values
int[0], int[1], and int[2] refer to. And then I remembered
the following instance method in BigInteger:

public BigInteger[] divideAndRemainder(BigInteger val)

which returns this/val and this%val in the array. The
user can infer the order of the array elements by the
order "divide" followed by "Remainder" in the method
name. In this spirit (or this convention), I have written
the following in accordance with your suggestion.

George

public class CharacterAnalysis2 {
public static int[] countVowelsConsonantsOthers(String str) {
int vowels = 0, consonants = 0, others = 0;
for (int j = 0; j < str.length(); j = j + 1) {
char ch = str.charAt(j);
if ( Character.isLetter(ch) ) {
switch(ch) {
case 'a': case 'e': case 'i': case 'o': case 'u':
case 'A': case 'E': case 'I': case 'O': case 'U':
vowels++;
break; //end cases for vowels

default:
consonants++;
//end ch is a letter but not a vowel
}//end switch statement
}
else { //ch is not a letter
others++;
}
}//end for loop
return new int[]{vowels, consonants, others};
}//end countVowelsConsonantsOthers method

/* Use the following method to test countVowelsConsonantsOthers(). */
public static void main(String[] args) {
StringBuffer buffer = new StringBuffer();
for (int j = 0; j < args.length; j = j + 1) {
buffer.append( (j == 0 ? "" : " ") + args[j] );
}
String str = buffer.toString();
int[] count = countVowelsConsonantsOthers(str);
System.out.println(str);
System.out.println( "Number of vowels = " + count[0] );
System.out.println( "Number of consonants = " + count[1] );
System.out.println( "Number of others = " + count[2] );
}//end main()
}//end class

 

[Ann]

I didn't intend to appear to diss your suggestion.
Indeed, I think it has merit. My problem with returning
an int array is that the user does not know which values
int[0], int[1], and int[2] refer to.

I'm sort of jumping into this discussion in the middle
but it seems to me that if you provide a method and the
user doesn't know what is returned that s/he would have
little incentive to invoke it.

 

[Ann]

: o ) What's that funny mark Frenchmen put above some e's?

(More below). George

It's a sideways umlaut.

 

[George W. Cherry]

I didn't intend to appear to diss your suggestion.
Indeed, I think it has merit. My problem with returning
an int array is that the user does not know which values
int[0], int[1], and int[2] refer to.

I'm sort of jumping into this discussion in the middle
but it seems to me that if you provide a method and the
user doesn't know what is returned that s/he would have
little incentive to invoke it.

Indeed. The problem is that the user knows that the
method returns the number of vowels, the number of
consonants, and the number of other characters in
the supplied string, but wouldn't know which element
of the returned array referred to which count.

George

 

[George W. Cherry]

It's a sideways umlaut.

Don't be vulgar.

 

[Chris Uppal]

int[] count = countVowelsConsonantsOthers(str);
System.out.println(str);
System.out.println( "Number of vowels = " + count[0] );
System.out.println( "Number of consonants = " + count[1] );
System.out.println( "Number of others = " + count[2] );

Now that /is/ execrable.

I'm sorry but there's no more polite word for it. I realise that you are only
exploring the options available to you, rather than seriously proposing this as
the best solution (as least I /hope/ that's the case ;-), but this passes
beyond sane program design.

-- chris

 

[Ann]

I didn't intend to appear to diss your suggestion.
Indeed, I think it has merit. My problem with returning
an int array is that the user does not know which values
int[0], int[1], and int[2] refer to.

I'm sort of jumping into this discussion in the middle
but it seems to me that if you provide a method and the
user doesn't know what is returned that s/he would have
little incentive to invoke it.

Indeed. The problem is that the user knows that the
method returns the number of vowels, the number of
consonants, and the number of other characters in
the supplied string, but wouldn't know which element
of the returned array referred to which count.

Since you are the program author, you can tell her on the phone.