e.setCancelled should return the actual cancel status

[_Iuri]

Supposing that we want to exit from a cancelled event function by us, we need to do:

e.setCancelled(true);

if(e.isCancelled())
    return;

Being that we could do:

if(e.setCancelled(true))
    return;

This case gets more sense with a true comparison case:

ItemStack theResult = e.getSlot(23);

if(e.setCancelled(theResult != null)) /* If the item is not null, cancel the event and exit quickly */
    return;

Also, this will be biutifier than:

ItemStack theResult = e.getSlot(23);

if(theResult != null)
{
    e.setCancelled(true);
    return;
}

Or

ItemStack theResult = e.getSlot(23);

e.setCancelled(theResult != null);

if(e.isCancelled())
    return;

What do you think?

 

[_Iuri]

This could be implemented with a new function just for Paper called "cancelled([status])" or something like that.

if(e.cancelled(true))
    return;

if(e.cancelled())
    return;

if(e.cancelled(false))
    return; // Never will be.

 

[electronicboy]

setCancelled is just a setter, there is no logic outside of a singular event in the API which will behave any differently, so this has generally 0 purpose

 

[_Iuri]

This kind of little modifications has its purpose: write less, more usefull and beauty code. The structure of an api or framework should be as worked as its programming.

Suppose this: We want to disable or enable a buttom from a window. How we would write the code? Should we create a function for enable it and another one for disable it?

class Button
{
    boolean isEnabled= true;
    
    void enable()
    {
        isEnabled= true;
    }
    
    void disable()
    {
        isEnabled= false;
    }
}

Or just one for both cases?

class Button
{
    boolean isEnabled = true;
    
    void enabled(boolean status)
    {
        isEnabled = status;
    }
}

And should we have two functions, one for set the status and another one to get the status?

class Button
{
    boolean isEnabled = true;

    void enabled(boolean status)
    {
        isEnabled = status;
    }

    boolean isEnabled()
    {
        return isEnabled;
    }
}

or just one?

class Button
{
    boolean isEnabled = true;

    boolean enabled(boolean status)
    {
        return isEnabled = status;
    }
}

Well, that example could not work with Java
Should be:

class Button
{
    boolean isEnabled = true;
    
    boolean enabled()
    {
        return isEnabled;
    }
    
    boolean enabled(boolean status)
    {
        return isEnabled = status;
    }
}

Should we follow the POO conventions of a language with a poorly POO implementation? I don't think so

 

[electronicboy]

I just fail to see the benefits of having a patch that modifies 250+ classes all for the sake of a trivial feature that in the scheme of things offers no real gains, whatever you set cancelled to is what it's set to, so I don't exactly see how the modification here is justified, especially for behavior you could easily shove in a util method

 

[_Iuri]

I get you and i am good with the implementation of the util methods.

I just wanna highlight the concept of code reduction. Within the API exists a lot of functions with void returns which hinder us at the time of programming. A clear example of this whould be the "getItem" and "setItem" of the Inventory interface.

I have to do:

Inventory i;

i.setItem(0, new ItemStack(Material.GLASS));
i.setItem(45, new ItemStack(Material.DIRT));
i.setItem(32, new ItemStack(Material.LAVA_BUCKET));

When i could do:

Inventory i;

i.setItem(0, new ItemStack(Material.GLASS)).setItem(45, new ItemStack(Material.DIRT)).setItem(32, new ItemStack(Material.LAVA_BUCKET));

Or, ideally:

Inventory i;

if(i.item(0, new ItemStack(Material.GLASS)).item(45, new ItemStack(Material.DIRT)).item(32, new ItemStack(Material.LAVA_BUCKET)).item(0).getType().equals(Material.GLASS))
{
    /* Do Something */
}

 

[_Pesto]

Please take a look at the Builder Design pattern