4
\$\begingroup\$

I am a beginner in Java. I have created this small project in Java with the inheritance concept. Here the Human will fill, pour and drink water. There are three classes: Glass, Jug and Human. Let me know how you think it is!

Glass class:

public class Glass {

    private float capacity = (float)0.5;
    private float quantity = (float)0.0;

    Glass(){

    }

    Glass(float capacity){
        this.capacity=capacity;
    } 

    public float getQuantity() {
        return quantity;
    }

    public float getCapacity(){
        return capacity;
    }

    public void setCapacity(float quantity){
        this.quantity = quantity;
    }

    public void fill(float c){
        this.quantity += c;
    }

    public void status(){
        System.out.print("Glass::: ");
        if(capacity == quantity){
            System.out.println("Full");
        }
        else if(capacity > quantity){
            System.out.println("Have " + quantity + " units");
        }
        else if(capacity < quantity){
            System.out.println("Overflowed");
        }else if(quantity == 0.0){
            System.out.println("Empty");
        }
    }
}

Jug Class:

public class Jug {
    private float capacity = (float)3.0;
    private float quantity = (float)0.0;

    Jug(){}

    Jug(float capacity){
        this.capacity = capacity;
    }

    Jug(float capacity,float quantity){
        this.capacity = capacity;
        this.quantity =  quantity;
    }

    public float getCapacity(){
        return capacity;
    }

    public void setCapacity(float quantity){
          this.quantity = quantity;    
    }

    public void fill(float q){
        quantity = q; 
    }

    public void pour(Glass g,float q){
        g.fill(q);
        this.quantity -= q;    
    }

    public void status(){
        System.out.print("Jug::: ");
        if(capacity == quantity){
            System.out.println("Full");
        }
        else if(capacity > quantity){
            System.out.println("Have " + quantity + " units");
        }
        else if(capacity < quantity){
            System.out.println("Overflowed");
        }else if(quantity == 0.0){
            System.out.println("Empty");
        }
    }

Human Class:

public class Human {
    private String name;
    private float d= 0;
    Human(){

    }
    Human(String name){
        this.name=name;
    } 

    public String getName(){
        return name;
    }

    public void setName(String name){
        this.name= name;
    }

    public void fillGlass(Glass g, Jug j,float quantity){    
        j.pour(g, quantity);    
    }

    public void fillJug(Jug j,float quantity){
        j.fill(quantity);
    }

    public void drink(Glass g, float quantity){
        g.setCapacity(-quantity);
         d +=quantity; 
    }

    public void status(){
        System.out.println(name + "consumed " + d + " units");
    }
}

Main Class:

 public static void main(String args[]){

    Jug j = new Jug(5.0f);
    Glass g = new Glass();

    Human m = new Human("Mickey Mouse");

    m.fillJug(j, 4.0f);
    m.fillGlass(g, j, 0.4f);
    m.drink(g, 0.2f);
    g.status();
    j.status();
    m.status();
}
\$\endgroup\$
6
  • 6
    \$\begingroup\$ You're not using inheritance at all. You probably meant for Jug to be extending Glass. \$\endgroup\$ – dustytrash Dec 13 '19 at 15:42
  • 2
    \$\begingroup\$ Exactly! Or maybe you could create a Container class, and have both Jug and Glass extend it. \$\endgroup\$ – cliesens Dec 13 '19 at 16:38
  • 2
    \$\begingroup\$ To me, data member accessors are a code smell. \$\endgroup\$ – greybeard Dec 13 '19 at 17:53
  • 4
    \$\begingroup\$ The Human could even be considered a container that has a capacity, and can be filled and (ahem) emptied. Oh, and "Mickey Mouse" is not a Human. \$\endgroup\$ – AJNeufeld Dec 13 '19 at 20:33
  • \$\begingroup\$ @cliesens Jug and Glass should only extend that class if they provide different behavior or otherwise need to be distinguished. In all other cases, just saying this glass is a container is enough, instead of saying all glasses are containers. \$\endgroup\$ – Roland Illig Dec 14 '19 at 17:37
3
\$\begingroup\$

Thanks for sharing your code.

As the comments tell, your code does not actually use inheritance, but that is only a feature of OOP, not a precondition.

OOP doesn't mean to "split up" code into random classes with random inheritance relationships.

The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.

Doing OOP means that you follow certain principles which are (among others):

  • information hiding / encapsulation
  • single responsibility
  • separation of concerns
  • KISS (Keep it simple (and) stupid.)
  • DRY (Don't repeat yourself.)
  • "Tell! Don't ask."
  • Law of demeter ("Don't talk to strangers!")

Inheritance comes into play when we need to modify a classes behavior. That is: one (or more) methods of the derived class get implementations, that differ from the implementation of the same method in the parent class.

Following this guidelines and having no further requirement that your code examples the suggestion of @AJNeufeld would be the best OOish solution: having one class Container with different configurations for glass, jug.

 public class Container {
    private String name;
    private float capacity;
    private float quantity = (float)0.0;


    Container(String name, float capacity){
        this.name =  name;
        this.capacity = capacity;
    }

    public float getCapacity(){
        return capacity;
    }

    public void setCapacity(float quantity){
          this.quantity = quantity;    
    }

    public void fill(float q){
        quantity = q; 
    }

    public void pour(Container g,float q){
        g.fill(q);
        this.quantity -= q;    
    }

    public void status(){
        System.out.print(name+"::: ");
        if(capacity == quantity){
            System.out.println("Full");
        }
        else if(capacity > quantity){
            System.out.println("Have " + quantity + " units");
        }
        else if(capacity < quantity){
            System.out.println("Overflowed");
        }else if(quantity == 0.0){
            System.out.println("Empty");
        }
    }
}

A human has a diferent behavior that a Container. Therefore it would need a class of its own as you did. But it would be simplyfied be the new approach. It only needs one method to fill a Container, not a single method for any.

public class Human {
    private String name;
    private float d= 0;
    Human(){

    }
    Human(String name){
        this.name=name;
    } 

    public String getName(){
        return name;
    }

    public void setName(String name){
        this.name= name;
    }

    public void pour(Container g, Container j,float quantity){    
        j.pour(g, quantity);    
    }

    public void fill(Container j,float quantity){
        j.fill(quantity);
    }

    public void drink(Container g, float quantity){
        g.setCapacity(-quantity);
         d +=quantity; 
    }

    public void status(){
        System.out.println(name + "consumed " + d + " units");
    }
}

This way the program is more flexible.

eg our original Program the human can only transfer content from a a jug to a glass, but not vize versa:

m.fillGlass(g, j, 0.4f);

With the new approach it can be dome in both directions:

Container j = new Container("Jug",5.0f);
Container g = new Container("Glass",1.0f);

Human m = new Human("Mickey Mouse");

m.fill(j, 4.0f);
m.pour(g, j, 0.4f);
m.pour(j, g, 0.2f);

And we can also use new types of containers without doing any change:

Container coffeCan = new Container("CoffeCan",2.0f);
Container cup = new Container("Cup",0.2f);
Container mug = new Container("Mug",0.4f);

Human me = new Human("Mickey Mouse");

me.fill(coffeCan, 2.0f);
me.pour(coffeCan, cup, 0.15f);
me.pour(coffeCan, mug, 0.35f);

General critic

Your code has some issues I'd like to address:

Avoid unnecessary mutability

Your classes Jug and Glass have a mutable property capacity. In real life it is quite unlikely that the capacity of a Jar or a Glass changes (significantly) during its lifetime. the same should be true for this Java objects during the runtime of the program. So you should make this properties immutable by apllying the final key word. Of cause this implies that you set it in (any) constructor and remove all setter methods:

 public class Container {
    private final String name;
    private final float capacity;
    private float quantity = (float)0.0;


    Container(String name, float capacity){
        this.name =  name;
        this.capacity = capacity;
    }
    // ...
}

Naming

Finding good names is the hardest part in programming. So always take your time to think carefully of your identifier names.

Single letter and abbreviated names

Avoid single letter and abbreviated names. Although this abbreviation makes sense to you (now) anyone reading your code being not familiar with the problem has a hard time finding out what this means.

If you do this to save typing work: remember that you way more often read your code than actually typing something. Also for Java you have good IDE support with code completion so that you most likely type a long identifier only once and later on select it from the IDEs code completion proposals.

Don't surprise your readers

A name of a method should clearly state what the method does.

In your code you have:

class Glass /* in class Jug too*/ {
    // ..
    public void setCapacity(float quantity){
          this.quantity = quantity;    
    }
    // ...
}

Here it is obvious, that the implementation is different from what the method name implies.

\$\endgroup\$
3
  • \$\begingroup\$ Is there any good reason that your code is formatted so inconsistently and uses an uncommon spacing style? \$\endgroup\$ – Roland Illig Dec 14 '19 at 12:58
  • \$\begingroup\$ @RolandIllig I just copied most of the code and I edit it here only, not in an IDE where I'd apply the auto formatter... \$\endgroup\$ – Timothy Truckle Dec 14 '19 at 13:31
  • \$\begingroup\$ @TimothyTruckle Boss, I understood all my issues about the project. I will carry on coding following your advice. \$\endgroup\$ – Jency Dec 15 '19 at 12:43
2
\$\begingroup\$

In addition to the other answers:

Floating-point types

You have this line in your code:

private float capacity = (float)0.5;

It seems strange that you have to tell the computer that 0.5 is indeed a (float) value, on the right side of the =. To fix this, there are two possibilities:

  1. If you are in a setting where performance is the most important goal of your code, keep using the float type but instead of (float) 0.5 just write 0.5f.

  2. In all other situations use the double type instead of float. Then you can write:

private double capacity = 0.5;

This is much cleaner and more idiomatic.

Source code layout

Currently your code is quite condensed: if(cond){. It's usual to write spaces between most of these program elements. You don't have to do that yourself, that's the job for your integrated development environment (IDE). In IntelliJ, just press Ctrl+Alt+L, in Eclipse press Ctrl+Shift+F, and you're done.

Floating-point accuracy

In a test run of your program, you should fill a glass with 0.3f. Then, take 0.2f away, and then again, take 0.1f away. You would probably expect now that the glass is empty. But it isn't. Your program tells you that the glass still contains 7.4505806E-9 (if you are using float), or even -2.7755575615628914E-17 (if you are using double). The latter means your glass is less than empty, which is not possible physically.

Welcome to the tricky fields of floating-point arithmetics, which can be surprising in many situations. Therefore, financial applications typically use integers instead of floating point numbers (they just say 123 cents instead of 1.23 dollars).

\$\endgroup\$
2
  • 1
    \$\begingroup\$ We should also point out that floating point primitive types are not accurate and should only be used at all if speed of the application is more valuable than the accuracy of the results. \$\endgroup\$ – Timothy Truckle Dec 14 '19 at 13:35
  • 2
    \$\begingroup\$ Thanks, I added a paragraph about the floating-point accuracy. \$\endgroup\$ – Roland Illig Dec 14 '19 at 17:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.