Task Vehicles done.#2
Task Vehicles done.#2yarikpavlin wants to merge 8 commits intoChangeRequest:masterfrom yarikpavlin:master
Conversation
| StringBuilder sb = new StringBuilder(); | ||
| if (Character.isDigit(value.charAt(0)) || value.startsWith("-")) | ||
| sb = sb.append(value.charAt(0)); | ||
| for (int i = 1; i < value.length(); i++) { |
There was a problem hiding this comment.
As far as I see, your implementation will correctly process smth like: G123 - where first character will be any symbol.
| public boolean isEven() { | ||
| //TODO: implement me | ||
| return false; | ||
| return this.value % 2 == 0; |
There was a problem hiding this comment.
As you can see, you have code duplication: there is no need to implement same method 2 times, as you can just call static method from non-static.
Also, assume, that you'll need to add some condition to this implementation, for example: isEven should return true if value is equal to 673 - you'll need to change your implementation 2 times: first one in static method and second one in non-static method.
Same comment about isOdd and isPrime implementations.
| this.make = make; | ||
| this.model = model; | ||
| this.maximumHeightFeet = maximumHeightFeet; | ||
| } |
There was a problem hiding this comment.
You have got a lot of duplicated code: manufacturedYear, make and model as parameters of Airplane AND Car AND Boat, so it'll be better to move them into some abstract predecessor.
I'll not duplicate this comment, but it also applies to Bot and Car implementations.
|
|
||
| @Override | ||
| public String toString() { | ||
| return "This airplane is a " + getManufacturedYear() + " " + this.getMake() + " " + this.getModel() + " that can reach " + |
There was a problem hiding this comment.
You now know how to use String.format and StringBuilder - please use them instead of raw concatenation.
| return false; | ||
| if (this == obj) | ||
| return true; | ||
| if (this.getMaximumHeightFeet() + 1000 != ((Airplane) obj).getMaximumHeightFeet() || this.getMaximumHeightFeet() - 1000 != ((Airplane) obj).getMaximumHeightFeet()) |
There was a problem hiding this comment.
It's not correct implementation.
obj.maximumHeightFeet should be between this.maximumHeightFeet+-1000.
So, if obj.maximumHeightFeet == this.maximumHeightFeet+500 - it should also return true.
And for obj.maximumHeightFeet == this.maximumHeightFeet-504 it should return true.
But it should return false for obj.maximumHeightFeet == this.maximumHeightFeet-1001 and obj.maximumHeightFeet == this.maximumHeightFeet+1001
| @Override | ||
| public boolean equals(Object obj) { | ||
| obj = (Airplane) obj; | ||
| if (obj == null) |
There was a problem hiding this comment.
First check should be null-check.
After that - you should check if currect object is really Airplane and only then re-assign value.
Also, do not assign anything to the argument - create new object.
| public String toString() { | ||
| if (this.motorized) | ||
| return "This boat is a " + this.getManufacturedYear() + " " + this.getMake() + " " + this.getModel() + " (with motor)."; | ||
| return "This boat is a " + this.getManufacturedYear() + " " + this.getMake() + " " + this.getModel() + "."; |
There was a problem hiding this comment.
Same about toString method as for Airplane
| obj = (Boat) obj; | ||
| if (this.toString() == obj.toString()) | ||
| return true; | ||
| return false; |
There was a problem hiding this comment.
It's not correct implementation also.
there should be:
- null-check
- classes check
- links check (check if obj == this)
- check for some specific parameters (for boat you should only check if
obj.motorized == this.motorized).
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| if (obj instanceof Boat) |
| return false; | ||
| if (this == obj) | ||
| return true; | ||
| if (this.getHorsepower() + 10 != ((Car) obj).getHorsepower() || this.getHorsepower() - 10 != ((Car) obj).getHorsepower()) |
There was a problem hiding this comment.
Here is the same principle as in Airplane
| package school.lemon.changerequest.java.container; | ||
|
|
||
| public class ContainerImpl implements Container{ | ||
| private int INITIAL_ARRAY_SIZE = 10; |
|
|
||
| public ContainerImpl() { | ||
| this.array = new int[Container.INITIAL_ARRAY_SIZE]; | ||
| this.size = 0; |
There was a problem hiding this comment.
size will be initialized as 0 by default
| /** | ||
| * Created by Yaroslav Pavlinskiy on 05.12.2016. | ||
| */ | ||
| public abstract class Vehicle { |
There was a problem hiding this comment.
this class could implement IVehicle and you should move all getter/setters here, so that you won't duplicate code and could also make all variables private.
2 participants
Other tasks will be made soon