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

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Apr 27, 2016 at 22:56
\$\endgroup\$

1 Answer 1

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 WayTM 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.

answered Apr 28, 2016 at 2:11
\$\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\$ Commented Apr 28, 2016 at 12:08
  • \$\begingroup\$ And I already use namespace aliases, In the frontend everything can be accessed by using the 'dx' alias. :) \$\endgroup\$ Commented Apr 28, 2016 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\$ Commented Apr 28, 2016 at 18:45

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.