Add the possibility to change the addressRange maxLen with options.

[Hurtsich]

Hello, we have a national service provider that use a custom protocol based on SMPP.
We need to have the possibility to set the max length for the address range.

I made the change using the option pattern (like the change on the connector to inject the address range)
This way we open the lib but do not break the public interface.

[laduchesneau]

Hi, I haven't looked over the code yet, but if I may propose another solution to this problem....lets make all Address fields exportalable. We don't need mutators and default TON/NPI will both be 0 anyway.

I would delete it all and just add a validator function

// Address smpp address of src and dst.
type Address struct {
	Ton     byte
	Npi     byte
	Addr string
}

// isValid verifies the Address, return a error if not valid
func (c *Address) IsValid() error{
	if len(c.Addr) > data.SM_ADDR_LEN {
		return fmt.Errorf("address len exceed limit. (%d > %d)", len(c.Addr), data.SM_ADDR_LEN)
	}
	return nil
}

Unless there is a specific reason we need getter and setter, that I am not seeing.

We would still keeps the marshal, unmarshal and string functions

This would be a breaking change, but a good one. If this solution is accepted, I would also recommend the same change for AddressRange.

[linxGnu]

@Hurtsich
Simplest solutions:

• Solution 1: Exposing AddressRange field
• Solution 2: implement function like this:

func (c * AddressRange) ForceSetAddressRange(addr string){
     c.addressRange = addr
}

Just it if you want to have custom addrRange without validation.

[laduchesneau]

I know we are talking about AddressRange, but both AddressRange and Address have the same issue. If we "fix" one, the other should also be "fixed".

• Solution 1: Exposing AddressRange field

• Solution 2: implement function like this:

I do love the simplicity of solution 2, but it doesn't really resolve the root cause of the problem.

With exported fields, all these functions are not needed.

func NewAddress() Address {}
func NewAddressWithAddr(addr string) (a Address, err error) {}
func NewAddressWithTonNpi(ton, npi byte) Address {}
func NewAddressWithTonNpiAddr(ton, npi byte, addr string) (a Address, err error) {}
func (c *Address) SetAddress(addr string) (err error) {}
func (c *Address) SetTon(ton byte) {}
func (c *Address) SetNpi(npi byte) {}
func (c *Address) Address() string {}
func (c *Address) Ton() byte {}
func (c *Address) Npi() byte {}

func NewAddressRange() AddressRange {}
func NewAddressRangeWithAddr(addr string) (a AddressRange, err error) {}
func NewAddressRangeWithTonNpi(ton, npi byte) AddressRange {}
func NewAddressRangeWithTonNpiAddr(ton, npi byte, addr string) (a AddressRange, err error) {}
func (c *AddressRange) SetAddressRange(addr string) (err error) {}
func (c *AddressRange) SetTon(ton byte) {}
func (c *AddressRange) SetNpi(npi byte) {}
func (c *AddressRange) AddressRange() string {}
func (c *AddressRange) Ton() byte {}
func (c *AddressRange) Npi() byte {}

Dont take this the wrong way....but I think of Java when I look at this code. :)

[linxGnu]

@laduchesneau
yeah, I understand your concern. I think best practice here is: only "open" if needed/on user request.
In this way, we balanced between maintenance effort and user need.

[Hurtsich]

@linxGnu If you're OK with breaking the public interface, i think just exposing the fields on AddressRange is fine.

[Hurtsich]

Ping, I updated the PR with the changes that we discussed. I'll let you merge if it's okay :)

[linxGnu]

LGTM. Thank you very much.