Finance | Project Operations, Human Resources, ...

Validate method

(6) Share

Report

Posted on by ..

1,907

Hi,

Normally, the validate method should return true and false. But inside this method i'm always throwing errors in case of failure, so if there was an error in the valdiate, it will never check the if condition if(this.validateTable1).

So how would you re-write this?

    if(this.validateTable1(table1))
    {
       //logic
    }

    private boolean validateTable1(Table1 _table1)
    {
        if (!_table1)
        {
            throw Error("@AAA:NotFound");
        }

        if (_table1.Fieldx != Enum::A)
        {
            throw Error(strFmt("@AAA:NotA", _table1.Id));
        }
       
        return true;
    }

would you do it like this?, i mean just call the validate without if? and make the validate return type as void? or how would you do this? (I just would like to know how will u do it)


    this.validateTabl1(table1))
    //logic

    private void validateTable1(Table1 _table1)
    {
        if (!_table1)
        {
            throw Error("@AAA:NotFound");
        }

        if (_table1.Fieldx != Enum::A)
        {
            throw Error(strFmt("@AAA:NotA", _table1.Id));
        }
       
    }

I have the same question (0)

All responses (9)

Answers (0)

Martin Dráb 236,330 Most Valuable Professional on at

Like (1)

Report

Validate method

The design pattern of validate* and check* method is that they return boolean. If you don't follow this pattern, please use another name to avoid confusion.

For me, the main problem with your approach is that you always stop on the first error. You don't tell users "there are these five problems with your record...", you tell them about the first one, let them fix it and try again, then you reveal the second problem and so on. It makes users' lives much more difficult.

Was this reply helpful? Yes No
Suggested answer

Monica Andrea Perez 12 on at

Like (2)

Report
Validate method

Hello, how are you?
If you have a method that validates, I recommend that you return a boolean, that is to say that you perform the validation and that the variables return a true or false and at the end of your method you return it. Why?
because the throw error immediately sends you a warning and will not go through the other validations, and sometimes it is annoying for one as a user to be filled with error messages.

For example:

private boolean validateTable1(Table1 _table1) { if (!_table1) { ok=true; } if (_table1.Fieldx != Enum::A) { ok=true; } return ok; }
Best regards.
Moni

Was this reply helpful? Yes No
Suggested answer

Martin Dráb 236,330 Most Valuable Professional on at

Like (2)

Report
Validate method

Well, this code has too many problems. It won't compile (it tries to assign a value to return statement, it doesn't return a value in all cases), it never returns false, it returns true when a check fails (while it should return false), it returns from a method immediately after each validation (which you attempted to fix)...

The correct implementation looks like this:

private boolean validateTable1(Table1 _table1) { boolean ok = true; if (!_table1) { ok = checkFailed("@AAA:NotFound"); } if (ok && _table1.Fieldx != Enum::A) { ok = checkFailed(strFmt("@ANNE:NotA", _table1.Id)); } return ok; }

Note that in this particular case, I check Fieldx only if the first check succeeded, because there is no point in checking correctness of a non-existing record. But if there were further checks, they should run regardless of whether Fieldx is A or not.

Was this reply helpful? Yes No
.. 1,907 on at

Like (1)

Report
Validate method

Hi Martin,

Thanks alot for your reply.

Question: Regarding your sentence "But if there were further checks, they should run regardless of whether Fieldx is A or not"

Then would you do it like this?

private Table1 updateTable1(Table1Contract _table1Contract) { Table1 table1; ttsbegin; select forupdate firstonly Id from table1 where table1.Id == _table1Contract.parmId(); if(this.validate(table1)) { // logic to update certain fields table1.update(); } else { throw Exception::Error; // where they will be a catch that will gather all error messages from the infolog } ttscommit; return table1; } private boolean validate(Table1 _table1) { boolean ret = true; if (!_table1) { ret = checkFailed("@AAA:NotFound"); } if (ret) { if(_table1.Fieldx != Enum::A) { ret = checkFailed(strFmt("@AAA:NotA", _table1.Id)); } if(_table1.Fieldy != Enum::C) { ret = checkFailed(strFmt("@AAA:NotC", _table1.Id)); } } return ret; }

OR this?

private Table1 updateTable1(Table1Contract _table1Contract) { Table1 table1; ttsbegin; select forupdate firstonly Id from table1 where table1.Id == _table1Contract.parmId(); if(!table1) { throw error("@AAA:NotFound"); } if(this.validate(table1)) { // logic to update certain fields table1.update(); } else { throw Exception::Error; // where they will be a catch that will gather all error messages from the infolog } ttscommit; return table1; } private boolean validate(Table1 _table1) { boolean ret = true; if(_table1.Fieldx != Enum::A) { ret = checkFailed(strFmt("@AAA:NotA", _table1.Id)); } if(_table1.Fieldy != Enum::C) { ret = checkFailed(strFmt("@AAA:NotC", _table1.Id)); } return ret; }

Was this reply helpful? Yes No
Martin Dráb 236,330 Most Valuable Professional on at

Like (1)

Report

Validate method

Do you have a reason not to use the standard method intended for validations before a record saving: validateWrite() (on Table1)?

Was this reply helpful? Yes No
.. 1,907 on at

Like (1)

Report
Validate method

Hi Martin,

Do you mean to add my logic to the validateWrite method via extensions? and do this instead:
if(table1.validateWrite()) { // logic to update certain fields table1.update(); }

The validation is only related when the code is called in my class, it shouldn't be called in other places.

but now does that mean that i should use both in my class?
if(this.validate() && table1.validateWrite()) { // logic to update certain fields table1.update(); }

and can you please answer my previous question about which way is better?

Was this reply helpful? Yes No
Martin Dráb 236,330 Most Valuable Professional on at

Like (1)

Report

Validate method

If your code shouldn't be called from other places, then it indeed doesn't belong to validateWrite(), but you must call validateWrite() before calling update(), otherwise you'd miss all other validations and you could save invalid data. Don't forget to rename validate() to something more descriptive.

Regarding your previous question, what is better depends on what you want. If your method should be called only when updating an existing record, calling it without _table1 is be wrong and it checking in your assumptions makes your code more safe and intent-revealing. If it's valid, you must decide whether your later validations make sense without _table1. If so, you don't want to check for _table1 being filled in (and vice versa).

Was this reply helpful? Yes No
.. 1,907 on at

Like (1)

Report
Validate method

Hi Martin,

As you can see in the code, if there is no _table1 record, then there is nothing to update and i already throw an error.

My question was, is it better to check table1 outside the validate method or inside is it?
if(table1)
{
this.validate(table1);
}

or this.validate(table1) -- where inside it there will be a check for if(!table1)

Or are you saying, that it's neither and that i should be doing this logic below outside the updateTable1 method (before calling it) and not inside it in the first place?

select forupdate firstonly Id from table1 where table1.Id == _table1Contract.parmId(); if(!table1) { throw error("@AAA:NotFound"); } this.updateTable1(table1);

Was this reply helpful? Yes No
Martin Dráb 236,330 Most Valuable Professional on at

Like (1)

Report

Validate method

There is no universally better solution - it depends on the particular situation.

Was this reply helpful? Yes No

Join the conversation

Community site session details

Validate method

Helpful resources

News and Announcements

Quick Links

Responsible AI policies

Abhilash Warrier – Community Spotlight

Congratulations to the September Top 10 Community Leaders!

Leaderboard > Finance | Project Operations, Human Resources, AX, GP, SL

Featured topics

Product updates

Community site session details

Validate method

Helpful resources

News and Announcements

Quick Links

Responsible AI policies

Abhilash Warrier – Community Spotlight

Congratulations to the September Top 10 Community Leaders!

Subscribe to this forum!

Select categories

Leaderboard > Finance | Project Operations, Human Resources, AX, GP, SL

Featured topics

Product updates