Code Java Bad Smells Questions and recursive question?

Mutant

Code Bad Smells Questions and recursive question?
These are three questions I got wrong in my last exam for programming, both are on code bad smells

Question 1

Consider the class excerpt below

public class Student{
private String id;
private Course course;
private String address_Street;
private String address_Town;
private String address_County;
private String grant_Authority;
private float grant_Value;
.........Methods.......
}
Name the code bad smell it contains and show the relevant code excerpts after refactoring.

Question 2

Consider the computeTotal() method of the ShoppingCart class below.

public class ShoppingCart{
List <Product> contents;

public double compareTotal(){
double total = 0.0;
for(Product p : products){
if (p.getQuantity()) > 100)
total += (p.getPrice() * p.getQuantity());
}
return total;
}

Name the code bad smell it contains and show the relevant code excerpts after refactoring.

Question 3
Write a recursive implementation of the method below:

public int countOccurences(String s, char c){
int result = 0;
if (s.length() != 0; i < s.length(); i++){
if (s.charAt(i) == c)
result ++;
}
}
return result;
}

montreal2011

Q1.

No constructor?

Q2.

public double compareTotal()
// where is the comparison?

for(Product p : products contents){

if (p.getQuantity()) > 100)
// why does 100 matter?

total += (p.getPrice() * p.getQuantity());
// why is each item being multiplied by the total number items?

It all smells bad to me!

Q3.

for loop

Evil Phil

Q1: It's better to encapsulate address and grant details in their own classes.

public class Student{
	private String id;
	private Course course;
	private Address address;
        private Grant grant;
	//.........Methods.......
}

public class Address
{
	private String street;
	private String town;
	private String county;
        // ... methods ...
}

public class Grant  
{
	private String authority;
	private float value;
        // ... methods ...
}

Q2: Yeah, it's contents not products. Also why the < 100 ? Need some more context I guess.


Q3 (untested! :pac:) Recursion is about a method that calls itself rather than using a loop.

public int countOccurences(String s, char c){
	int result = 0;
	if(s.charAt(0) == c) {
		result++;
	}
	if(0 != s.length()) {
		// recursion occurs here
		result += countOccurences(s.substring(1), c);
	}
	return result;
}  

croo

Q2. Using a double for an amount value is always going to be problematic ... BigDecimal all the way!

The rest EvilPhil has addressed.

Mutant

Evil Phil wrote: »

Q1: It's better to encapsulate address and grant details in their own classes.

public class Student{
	private String id;
	private Course course;
	private Address address;
        private Grant grant;
	//.........Methods.......
}

public class Address
{
	private String street;
	private String town;
	private String county;
        // ... methods ...
}

public class Grant  
{
	private String authority;
	private float value;
        // ... methods ...
}

Q2: Yeah, it's contents not products. Also why the < 100 ? Need some more context I guess.


Q3 (untested! :pac:) Recursion is about a method that calls itself rather than using a loop.

public int countOccurences(String s, char c){
	int result = 0;
	if(s.charAt(0) == c) {
		result++;
	}
	if(0 != s.length()) {
		// recursion occurs here
		result += countOccurences(s.substring(1), c);
	}
	return result;
}  

Thanks for the replies
Just for question 1 do you know what the name of the bad smell is aswell

These were just questions in an exam that i got wrong and failed.

Kidchameleon

What exactly is "bad smells"??

Evil Phil

I guess you'd call it incorrect encapsulation. Did they actually use the expression "bad smell" in your exam or did you add it yourself?

Mutant

Evil Phil wrote: »

I guess you'd call it incorrect encapsulation. Did they actually use the expression "bad smell" in your exam or did you add it yourself?

its the exact way it was wrote in the exam, I copied it straight from the exam paper.

It has me completely flustered and this was a end of year college exam

Evil Phil

Where are you studying?

Mutant

Evil Phil wrote: »

Where are you studying?

Entertainment Systems(computer game design stream) in Waterford Institute of Technology

Why??

montreal2011

Evil Phil wrote: »

I guess you'd call it incorrect encapsulation. Did they actually use the expression "bad smell" in your exam or did you add it yourself?

It might be more a cohesion issue that solved by separating the address and grant into their own classes.

I never heard of "code bad smell" before! It's terrible grammar too! Bit of a sneaky term to put in a final exam if it hasn't been mentioned in class before! :pac:

henryporter

The 'Bad Smells' phrase are courtesy of Martin Fowler in his Refactoring book and refer to typical coding that can be improved through refactoring. For example the first question which is rightly identified as needing a separate class for common address variables is referred to as Long Parameter List requiring a 'Introduce Parameter Object' refactoring solution.

Mutant

henryporter wrote: »

The 'Bad Smells' phrase are courtesy of Martin Fowler in his Refactoring book and refer to typical coding that can be improved through refactoring. For example the first question which is rightly identified as needing a separate class for common address variables is referred to as Long Parameter List requiring a 'Introduce Parameter Object' refactoring solution.

Thanks for your answer what you are saying rings a bell, but we only got one quick lecture on the matter. Sorry for bothering you but do you think you could give the full answers on these questions then.

Question 1

Consider the class excerpt below

public class Student{
private String id;
private Course course;
private String address_Street;
private String address_Town;
private String address_County;
private String grant_Authority;
private float grant_Value;
.........Methods.......
}
Name the code bad smell it contains and show the relevant code excerpts after refactoring.

Question 2

Consider the computeTotal() method of the ShoppingCart class below.

public class ShoppingCart{
List <Product> contents;

public double computeTotal(){
double total = 0.0;
for(Product p : products){
if (p.getQuantity()) > 100)
total += (p.getPrice() * p.getQuantity());
}
return total;
}

Name the code bad smell it contains and show the relevant code excerpts after refactoring.

Any help with this is appreciated

Malice

I've seen it referred to as a "code smell" before but never a "code bad smell".

henryporter

Yes - the term bad smells raised a lot of guffaws in the lectures when they were being discussed.

In terms of solutions see http://sourcemaking.com/refactoring/introduce-parameter-object for question 1 and http://sourcemaking.com/refactoring/replace-parameter-with-method for question 2. These links provide the best explanation of the refactoring solutions required. The bad smell in question 2 is 'Long Method' (I think - in fact it isn't really too long a method but in terms of an academic example/solution you can't go wrong with that).

Presumably question 3 has nothing to do with bad smells but is purely a recursion question

amen

[PHP]private String county[/PHP]
not that important but since most countries have a defined list of counties I would have

[PHP]private int countyID[/PHP]

and have a list of counties.

Makes the spelling consistent and easier to do queries

Mutant

henryporter wrote: »

Yes - the term bad smells raised a lot of guffaws in the lectures when they were being discussed.

In terms of solutions see http://sourcemaking.com/refactoring/introduce-parameter-object for question 1 and http://sourcemaking.com/refactoring/replace-parameter-with-method for question 2. These links provide the best explanation of the refactoring solutions required. The bad smell in question 2 is 'Long Method' (I think - in fact it isn't really too long a method but in terms of an academic example/solution you can't go wrong with that).

Presumably question 3 has nothing to do with bad smells but is purely a recursion question

Thanks very much, your help is much appreciated

Evil Phil

montreal2011 wrote: »

It might be more a cohesion issue that solved by separating the address and grant into their own classes.

A much better answer, go with this one.

I've heard the phrase code smell being used on stackoverflow.com, but I'm surprised at it being used in an exam. I think asking a student to improve the following code excepts might be a more professional approach, but maybe that's just me.