olaven / arbeidskrav

School assigment

• ⚡ Curious programmer
• 🔭 Throwing away throwaway society at Tings
• ❄️ Continually trying to improve at cycling

arbeidskrav's Issues

MeterArchive.Move bug

arbeidskrav/V18_1_basic/MeterArchive.java

Lines 64 to 71 in 48d454a

	public boolean move(String identification, String newLocation) {
	Meter meter = fetch(identification);
	if (meter == null)
	return false;
	//the meter was fetched, starting to alter
	meter.setLocation(newLocation);
	return true;
	}

You are not checking whether the position you are moving the Meter to is already in use. If this is the case both Meters will share the same position, but only one will be retrievable from the MeterArchive.

I propose you check whether the position is already in use before you move the Meter, and if so, swap position with the other Meter than had the position you want to move to.

Put the build instructions on two lines

In /V18_1/besvarelse/README.md you write the two build commands on a single line. This might confuse people and render them unable to try the demo! I propose you stick them on two lines like so:

javac *
java Client

.fetch always returns null

arbeidskrav/V18_1_extra/src/main/java/arbeidskrav/MeterArchive.java

Lines 61 to 69 in 85be5d6

	public Meter fetch(String identification) {
	ArrayList<Meter> meters = castArrayListToMeters(storageManager.fetchAllData());
	for (Meter meter : meters) {
	if (meter.getIdentification() == identification){
	return meter;
	}
	}
	return null;
	}

``ìdentificationis never equal tometer.getIdentification()```.

Why are you checking for an empty string?

arbeidskrav/V18_2/Question.java

Lines 34 to 41 in 7b7ba59

	public void setText(String text){
	if(text.length() > 0)
	this.text = text;
	}
	public void setAnswer(String answer){
	if(answer.length() > 0)
	this.answer = answer;
	}

You could argue that it makes sense to verify that the string is not null, but I would argument strongly against an empty string.
Firstly, unlike null an empty string will not crash your program, and there are tons of valid reasons why you might want one.
Secondly, I think it is fairly safe to assume that the questions will not change after initialization, and if that is the case then the constructor is the only place where those setters are being used, so if a question is initialized with an empty string then the question will just be an empty string regardless!
Lastly; if questions indeed are immutable after initialization then the setters are entirely redundant altogether.

MeterArchive.fetch is ridiculously slow

arbeidskrav/V18_1_basic/MeterArchive.java

Lines 49 to 56 in d7fd8bf

	public Meter fetch(String identification) {
	for (Meter meter : meters) {
	if (meter.getIdentification() == identification) {
	return meter;
	}
	}
	return null;
	}

MeterArchive.fetch (and consequently all of the other methods in MeterArchive) has time complexity O(N) - this is ridiculously slow for a lookup! With as little as a couple of million meters in the archive the user would have to wait for seconds to do a lookup, and in that time they might as well go look themselves. Yuck!

There are approximately two solutions to this:

Change data-structure

Arrays are not meant for lookup, they are for fast access if you know the index of what you want. Consider switching from ArrayList to something like a HashMap that has O(1) lookup.
Have a look at the big o cheat sheet for the time complexity of different data-structures.

Be clever

If you have to store the Meters in an array, you can store them in sorted order (sorted on the reference number). Then when you want to look for a Meter you can play the number guessing game:
Take the middle meter out of the list and compare its reference number against the one you are looking for. If it is smaller/greater you repeat for the lower/upper half of the list. Continue doing this until you find an element that is equal. This has time complexity O(log N).