3
\$\begingroup\$

I want to improve my slider class as much as possible. Is this acceptable code for a slider?

I've tried to comment as much as I can. If you want to find out more about the Component class, check out my repository: dxLib.

slider.h:

#pragma once
#include "Component.h"

begin_UI // Namespace define
class Textbox;
class RichLabel;

class Slider : public Component
{
public:
    Slider( );

    void Paint( Window *sender, BasePainter *painter ) override;

    void KeyDown( Window *sender, KeyDownArgs &_Args ) override;

    void KeyUp( Window *sender, KeyUpArgs &_Args ) override;

    void KeyDownChar( Window *sender, KeyDownCharArgs &args ) override;

    void MouseMoved( Window *sender, MouseMovedArgs &args ) override;

    void MouseClicked( Window *sender, MouseClickedArgs &args ) override;

    void MouseReleased( Window *sender, MouseReleasedArgs &args ) override;

    __MATH Vector2 getDelta( ) const;

    void setMaxDelta( const float &delta );

    float getMaxDelta( ) const;

    void setWheel( const float &delta );

    float getWheel( ) const;

    void setWheelSize( const __MATH Vector2 &size );

    __MATH Vector2 getWheelSize( ) const;

    bool CollidesWheel( const __MATH Vector2 &position );

    bool inScrollableRegion( const __MATH Vector2 &cursor ) const;

    std::shared_ptr<Textbox> getTextbox( ) const;


private:
    void moveWheelToDelta( );
    __MATH Vector2 moved_, wheel_, wheelSize_, delta_;
    std::shared_ptr<Textbox> textbox_;
    bool changed_, dragging_;
    float maxDelta_;
};



end_UI

Implementation:

#include "Slider.h"
#include "Textbox.h"
#include "RichLabel.h"
#include "../Pen.h"

begin_UI

Slider::Slider()
    : Component( )
{
    // Textbox to change the value of the slider
    // by using a textbox instead of the wheel.
    textbox_ = std::shared_ptr<Textbox>( new Textbox( ) );
    textbox_->setUIID( String("Slider_component_textbox").hash( ) );
    textbox_->setText( "0%" );
    textbox_->setFilter( "1234567890" );
    textbox_->setRightOf( this );
    textbox_->setAllignedOf( this );
    textbox_->setSize( { 50, 30 } );
    textbox_->setLooseFocusKey( '\r' );
    textbox_->setAllignment( Allignment::Left );

    maxDelta_ = 100;

    // When the textbox gains focus remove the '%'
    // character, otherwise there will be parsing errors
    textbox_->OnGainFocus( ) += []( Component *sender )
    {
        auto text = sender->getText( );
        uint npos = text.find( '%' );
        if ( npos == String::bad )
            return;

        text.erase( npos, 1 );
        sender->setText( text );
    };

    // When we lose focus then add the '%' again 
    // and also recalculate the delta and move the wheel.
    textbox_->OnLostFocus( ) += [this]( Component *sender )
    {
        auto text = sender->getText( );
        uint npos = text.find( '%' );
        if ( npos == String::bad )
        {
            this->delta_ = { text.to<float>( ), text.to<float>( ) };
            text.push_back( '%' );
            sender->setText( text );
            if ( this->delta_.x > this->maxDelta_ || this->delta_.y > this->maxDelta_ )
            {
                text = to_string( (int)this->maxDelta_ ) + "%";
                delta_ = { maxDelta_, maxDelta_ };
                sender->setText( text );
            }

            this->moveWheelToDelta( );
            return;
        }

        text.erase( npos, 1 );
        this->delta_ = { text.to<float>( ), text.to<float>( ) };

        if ( this->delta_.x > this->maxDelta_ || this->delta_.y > this->maxDelta_ )
        {
            text = to_string( (int)this->maxDelta_ ) + "%";
            delta_ = { maxDelta_, maxDelta_ };
            sender->setText( text );
        }

        this->moveWheelToDelta( );
    };


    OnModified( ) += [this]( Component *sender )
    {
        this->changed_ = true;      
        this->textbox_->setStyle( style_ );
        this->textbox_->setFont( font_ );
    };

    changed_ = true;
}

void Slider::Paint(Window *sender, BasePainter *painter)
{
    static Pen inner, outer{ Colors::White, 1 };
    if ( !isVisible( ) )
        return;
    OnPrePaint( ).Invoke( this, painter );

    // Determine pos
    auto pos = determineRegion( );

    if ( changed_ )
    {
        delta_.x = (wheel_.x / (local_.size.x - wheelSize_.x)) * maxDelta_;
        delta_.y = (wheel_.x / (local_.size.y - wheelSize_.y)) * maxDelta_;

        if ( layout_ == Horizontal )
            textbox_->setText( to_string( (int)delta_.x ) + "%" );
        else
            textbox_->setText( to_string( (int)delta_.y ) + "%" );

        changed_ = false;
    }

    // Style
    auto style = getStyle( );
    auto color_inner = style.theme( ) == Dark ? 0xFF2C2C2C : 0xFFC8C8C8;
    auto color_outer = 0xFF4B4B4B;
    auto color_bar = decltype(color_inner)();

    if ( dragging_ )
        color_bar = style.style( );
    else
        color_bar  = 0x909090;

    painter->PaintRectOutlined( pos, inner, outer );
    painter->PaintRect( { pos.position + wheel_, wheelSize_ }, Pen( color_bar, 1 ) );

    textbox_->Paint( sender, painter );

    OnPostPaint( ).Invoke( this, painter );
}

void Slider::KeyDown(Window *sender, KeyDownArgs &args)
{
    textbox_->KeyDown( sender, args );
}

void Slider::KeyUp(Window *sender, __GRAPHICS KeyUpArgs & args)
{
    textbox_->KeyUp( sender, args );
}

void Slider::KeyDownChar(Window *sender, __GRAPHICS KeyDownCharArgs & args)
{
    textbox_->KeyDownChar( sender, args );
}

void Slider::MouseMoved(Window *sender, __GRAPHICS MouseMovedArgs & args)
{
    //If dragging
    if ( dragging_ && inScrollableRegion( args.position ) )
    {
        if ( moved_.x != 0 && moved_.y != 0 )
        {
            auto increment = (args.position - determined_.position) - (moved_ - determined_.position);

            if ( layout_ == Horizontal )
            { 
                wheel_.x += increment.x;
                if ( wheel_.x > (local_.size.x - wheelSize_.x) )
                    wheel_.x = (local_.size.x - wheelSize_.x);
                if ( wheel_.x < 0 )
                    wheel_.x = 0;
            }
            else
            { 
                wheel_.y += increment.y;
                if ( wheel_.y > (local_.size.y - wheelSize_.y) )
                    wheel_.y = (local_.size.y - wheelSize_.y);
                if ( wheel_.y < 0 )
                    wheel_.y = 0;
            }
        }
        moved_ = args.position;
        changed_ = true;
    }

    textbox_->MouseMoved( sender, args );
}

void Slider::MouseClicked(Window *sender, __GRAPHICS MouseClickedArgs & args)
{
    // If collides anywhere & pressing ctrl, then directly move
    // the wheel.
    if ( Collides( args.position ) && args.ctrl )
    {
        // Get the sub offset from mouse - determined_position.
        if ( layout_ == Horizontal )
            wheel_.x = (args.position.x) - determined_.position.x;
        else // |
            wheel_.y = (args.position.y) - determined_.position.y;
        this->changed_ = true;
    }

    // Collides with the wheel
    if ( CollidesWheel( args.position ) )
        dragging_ = true;
    else
    {
        dragging_ = false;
        moved_ = { 0, 0 };
    }

    textbox_->MouseClicked( sender, args );
}

void Slider::MouseReleased(Window *sender, __GRAPHICS MouseReleasedArgs & args)
{
    if ( hovering_ )
        OnMouseReleased( ).Invoke( this );
    dragging_ = false;
    clicking_ = false;
    moved_ = { 0, 0 };
    textbox_->MouseReleased( sender, args );
}

__MATH Vector2 Slider::getDelta() const
{
    return delta_;
}

void Slider::setMaxDelta(const float & _Delta)
{
    maxDelta_ = _Delta;
}

float Slider::getMaxDelta() const
{
    return maxDelta_;
}

void Slider::setWheel(const float & _Delta)
{
    if ( layout_ == Horizontal )
        wheel_.x = _Delta;
    else
        wheel_.y = _Delta;
}




float Slider::getWheel( ) const
{
    if ( layout_ == Horizontal )
        return wheel_.x;
    else
        return wheel_.y;
}

void Slider::setWheelSize(const __MATH Vector2 & _Size)
{
    wheelSize_ = _Size;
    changed_ = true;
}

__MATH Vector2 Slider::getWheelSize() const
{
    return wheelSize_;
}

bool Slider::CollidesWheel(const __MATH Vector2 & _Position)
{
    return _Position.Intersects( { determined_.position + wheel_, wheelSize_ } );
}

bool Slider::inScrollableRegion(const __MATH Vector2 & _Cursor) const
{
    return (layout_ == Horizontal ? 
           _Cursor.Intersects( { { determined_.position.x, 0 }, { determined_.size.x, 10000000 } } ) : 
           _Cursor.Intersects( { { 0, determined_.position.y }, { 10000000, determined_.size.y } } ) );
}

std::shared_ptr<Textbox> Slider::getTextbox() const
{
    return textbox_;
}


void Slider::moveWheelToDelta()
{
    if ( layout_ == Horizontal )
    {
        auto wheel = (delta_.x / maxDelta_) * (local_.size.x - wheelSize_.x);
        wheel_.x = wheel;
    }
    else
    {
        auto wheel = (delta_.y / maxDelta_) * (local_.size.y - wheelSize_.y);
        wheel_.y = wheel;
    }
}

end_UI

Preview:

Preview

\$\endgroup\$
4
\$\begingroup\$

Looks pretty neat. I only have a couple comments:

At first I was puzzled by those __SOMETHING macros, then I looked into your repo and they are actually aliasing a namespace:

#ifndef __MATH
#define __MATH ::dx::lib::Math::
#endif

This might seem like silly nitpicking, but it can actually be a serious problem. Names starting with double underscore are reserved in C++ (ref), but we can usually get aways with it because our code rarely uses a reserved name, except when it does! I would not be surprised at all to find that same __MATH macro defined in math.h or __MEMORY defined in the C++ memory header. You are walking on thin ice over there.

If you can't be bothered spelling out the names of the namespaces you've created, then you can alternatively define a shorthand namespace alias, which would be The Right Way™ of doing it.

Not being a huge fan of macros either, I'd prefer to see the explicit:

namespace dx { namespace lib { namespace Graphics { namespace UI {

Rather than begin_UI. Users might miss that detail and attempt to use the contents of the namespace directly in code, thinking that the class is defined globally, only to result in annoying compilation errors.


You've misspelled Allignment and Alligned. It is with one L as in setAlignment and setAlignedOf.

\$\endgroup\$
3
  • \$\begingroup\$ Ah yes, the defines, I knew it was a bad idea, I used it because in the start I was always changing the names of my namespaces, so I had to go through every single file and change the name of the namespaces, so I thought of using macros instead. I'll also fix the missspellings. Thanks! \$\endgroup\$ Apr 28 '16 at 12:08
  • \$\begingroup\$ And I already use namespace aliases, In the frontend everything can be accessed by using the 'dx' alias. :) \$\endgroup\$ Apr 28 '16 at 12:10
  • 1
    \$\begingroup\$ @PsychoBitch, inline namespaces might also be an option, depending on the case: en.cppreference.com/w/cpp/language/namespace \$\endgroup\$
    – glampert
    Apr 28 '16 at 18:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.