search

PileProject / drive

The drive project
Other
5 stars 4 forks source link

Fix a bug on validation for MAC address #67

Closed tiwanari closed 7 years ago

tiwanari commented 7 years ago

I found a small but fatal bug.

The validation of a MAC address is wrong. If the string is not set, "" will be returned not null.

tiwanari commented 7 years ago

Doesn't the validation work correctly too?

No, thank you for pointing out that!

Your suggestion is quite reasonable. Will do:)

tiwanari commented 7 years ago

I did:)

mandaiy commented 7 years ago

Ah, I meant that why not add the default value to the definition of MachinePreference like

@Table(name = "machine_preferences")
public class MachinePreferencesSchema {
    // ...
    @Key(name = "bluetooth_address")
    String macAddress = "";
    // ...

But calling getMacAddress() with providing a default argument also makes sense and might be enough. It's up to you whether adding the default value to the class definition or not.

tiwanari commented 7 years ago

Ah, sorry for my misunderstanding.. Hmm, in this case, both of them are reasonable but this kind of bugs is easily made and confusing. I guess this is a good time to start using guava which has useful methods like isNullOrEmpty . What do you think?

tiwanari commented 7 years ago

I think it's natural the method returns null (that's why the bug was made ) and returning "" as well (in the Java world). So, using the until method, we don't have to specify the default value from my viewpoint.

mandaiy commented 7 years ago

Introducing guava is fine to me. But I think there are a lot of lines we can refactor by using the library and I want to replace such lines at a time.

So can you close this pull request? And let's discuss the library and list up the lines in another issue or pull request.

tiwanari commented 7 years ago

okay, after modifying lines you mentioned, I'll close this and raise an issue