Which is better or does it matter? Two methods which does similar things or one method?

I still am learning and would like to know what would be considered better here .  This is all automation test code.   I did a code review and the person changed a few method's signature from void to String and added a String value to be returned.  There are four other methods which use this one method and would not do anything with the returned String value so I guess he did this for his one method which does use the String value.

So the method WAS


The method NOW



So the four methods which calls the method selectFirstCard() doesn't do anything with a String value so it's not breaking anything but I'm just curious if this really is the best way to handle a situation where you need what the method does but you need it to return something rather than the method to just do something?    Existing method does an action , you need that action but you also need it to return something.  So rather than create a new method that does the nearly identical steps AND returning a value doesn't break anything, so just change the method's signature and have it return what you want.


Example of a method that was using this

Welcome back

Do testing methods usually return a value? I may well be mistaken, but I would sooner stick to the void method and write another method to return a String.

Campbell Ritchie wrote:Welcome back

Do testing methods usually return a value? I may well be mistaken, but I would sooner stick to the void method and write another method to return a String.

Hey Thanks!  My learning path has taken me from learning Java to being moved from backend stuff to front end automation testing so now I had to learn Selenium, and Cucumber but oh wait!  We don't want to use Cucumber anymore so now we are doing everything in just Junit .  Now I'm taking a class in REST Assured ...  Oiy.  LOL.  So I'm always a newbie I think.   No complaints .  At least I have the opportunity .

The change made was to return the current URL and he's doing some validation there using junit .  I think MY instinct would to create a new method that would return the string and leave the existing method alone but I wasn't sure if my instincts were right since by returning the current URL value doesn't hurt anything .

No, Junit, at least, does NOT return anything from tests. If a test fails, it throws an Exception.

I can see where it might facilitate testing for the method UNDER test to return a result, but I don;'t recommend doing so, partly because sooner or later someone is likely to mis-use it in production.

Then again, going a "get" on a list doesn't normally cause UI effects, which is what the sample code seems to be expecting.  At a minimum, I'd expect to see the list-management code in a separate method from the UI-affecting code and tested seperately. Even if a higher-level method wsa used to mutually invoke them.

Going to an even higher level, in Model/View/Controller terms, it gives me the impression that Model and Controller are both part of the same class and that's not really how MVC should be working. It's one reason why I take people to task when they call their JSF backing beans "Controllers". They aren't. They're Modes, which may have connexion to business logic, but their Controllers are located elsewhere.

Tim Holloway wrote:No, Junit, at least, does NOT return anything from tests. If a test fails, it throws an Exception.

I can see where it might facilitate testing for the method UNDER test to return a result, but I don;'t recommend doing so, partly because sooner or later someone is likely to mis-use it in production.

Then again, going a "get" on a list doesn't normally cause UI effects, which is what the sample code seems to be expecting.  At a minimum, I'd expect to see the list-management code in a separate method from the UI-affecting code and tested seperately. Even if a higher-level method wsa used to mutually invoke them.

Going to an even higher level, in Model/View/Controller terms, it gives me the impression that Model and Controller are both part of the same class and that's not really how MVC should be working. It's one reason why I take people to task when they call their JSF backing beans "Controllers". They aren't. They're Modes, which may have connexion to business logic, but their Controllers are located elsewhere.

I'm not saying any of the methods I posted uses Junit.  We are moving from cucumber to Junit because from what I'm told want to use it in Gitlab's CICD pipeline to run these automation tests now.  These are just for testing and not code we are using in production.

We do have the methods in different classes.  

I'm just trying to understand if the modification the person made is consider best practice or doesn't really matter.  I wanted to say "Hey we should create a different method for what you want to do" but then I don't feel confident as to whether it mattered or not.

The comment about Junit was really more for Campbell's benefit and yes, there are lots of other test frameworks, although they generally have similar characteristics.

But in short, I don't recommend that UI code and application logic both exist in a common application method. Or, as a rule, even in the same class.

In a test method, it might be different. But in that case, I'd be doing assertions that the UI was doing what you told it to do and was responding with both the correct data and the correct events.

Tim Holloway wrote:. . . I don't recommend that UI code and application logic both exist in . . . the same class. . . .

I would agree there.