3
\$\begingroup\$

The send_sms_message method takes text as an argument. The length of text is unknown and can be 0 or as large as 1070 (after that we will have no space for the actual text, because multi-part message text will be so long).

The trick is that if text is larger then 160 characters, we need to split it into parts and to each part add text part 1 of 2.

The problem is that we don't know the length of the extra text. It can be 14 characters for part 1 of 2 or 16 for part 11 of 99 and so on.

This code can handle up to 99 parts. How can I make it generic to handle more parts without adding more conditions?

SMS_LENGTH = 160
MPM_SIZE_LONG = 16
MPM_SIZE_SHORT = 14
MPM_SHORT_LIMIT = 1314

def send_sms_message(text, to, from)
  unless text.length > SMS_LENGTH
    deliver_message_via_carrier(text, to, from)
  else
    parts = text.scan(/.{1,#{SMS_LENGTH - (text.length > MPM_SHORT_LIMIT ? MPM_SIZE_LONG : MPM_SIZE_SHORT)}}/)
    parts.to_enum.with_index(1) do |message_part, index|
      deliver_message_via_carrier("#{message_part} - Part #{index} of #{parts.length}", to, from)
    end
  end
end
\$\endgroup\$
6
  • \$\begingroup\$ It's worth noting that your current code will make 159-character messages for the first 9 parts of a text with 10 or more parts, because " - Part 9 of 10" is only 15 characters. \$\endgroup\$
    – Aaron
    Jul 1 '16 at 2:36
  • \$\begingroup\$ @Aaron Yeah, I know we are wasting 1 byte here. But it is the tiniest problem here. \$\endgroup\$ Jul 1 '16 at 2:44
  • \$\begingroup\$ Sorry, without adding more conditions or changing your approach, I don't see a way to support more messages or eliminate the occasionally 1-byte difference. Regex and String.scan just aren't dynamic enough, as far as I can tell. \$\endgroup\$
    – Aaron
    Jul 1 '16 at 2:55
  • 1
    \$\begingroup\$ @Aaron You are more than welcome to change the approach, this is why I posted code here. \$\endgroup\$ Jul 1 '16 at 3:11
  • \$\begingroup\$ Ah, when you said "without adding more conditions" I took that to also mean using this approach. \$\endgroup\$
    – Aaron
    Jul 1 '16 at 7:01
2
\$\begingroup\$

Some notes:

  • Is your code inside a module or a class, I guess? You can include it in the question.

  • As a general rule, favour the user of affirmative statements (if instead of unless).

  • If you spend enough time with a pencil and a paper, maybe you'd get to a nice formula get_total_parts(message_size, sms_size, parts_message_min_size), but it's not trivial, that's for sure. A pre-computing of the max_size (along with some extra info you'll need) is a bit tedious to write but fast. For example, what's the maximum size if you have a total count with exactly 3 digits (100-999)? (SMS_SIZE - PARTS_MSG_MIN_SIZE - 2) * 9 + (SMS_SIZE - PARTS_MSG_MIN_SIZE - 3) * 90 + (SMS_SIZE - PARTS_MSG_MIN_SIZE - 4) * 900. You get the idea.

I'd write:

module SMS
  SMS_LENGTH = 160

  PARTS_MESSAGE = " - Part %{n} of %{total}"

  SPLIT_INFO = (1..70).map do |ndigits|
    base_size = (PARTS_MESSAGE % {n: "1", total: "1"}).size 
    parts_per_digit = (0...ndigits).map do |n|
      (SMS_LENGTH - base_size - (ndigits + n - 1)) * (9 * (10 ** n))
    end
    {
      max_size: parts_per_digit.reduce(0, :+), 
      size_of_first_parts: parts_per_digit[0...-1].reduce(0, :+),
      min_parts: 10**(ndigits - 1) - 1, 
      msgsize_for_last_parts: (SMS_LENGTH - base_size - 2 * (ndigits - 1))
    }
  end

  def self.get_total_parts_for_long_message(text)
    info = SPLIT_INFO.detect { |h| text.size <= h[:max_size] } or 
      raise ValueError("Message text too large")
    info[:size_of_first_parts] + 
      Rational(text.size - info[:min_size], info[:msgsize_for_last_parts]).ceil
  end 

  def self.send_sms_message(text, to, from)
    if text.length <= SMS_LENGTH
      deliver_message_via_carrier(text, to, from)
    else
      total_parts = get_total_parts_for_long_message(text) 
      idx = 0

      (1..total_parts).each do |part_index|
        split_message = PARTS_MESSAGE % {n: part_index, total: total_parts}
        user_message_size = SMS_LENGTH - split_message.size
        message_text = text[idx, SMS_LENGTH - user_message_size]
        deliver_message_via_carrier(message_text + split_message, to, from)
        idx += user_message_size
      end
    end
  end

  def self.deliver_message_via_carrier(text, to, from)
    puts("Sending #{from} -> #{to}: #{text} - #{text.size} bytes")
  end
end
\$\endgroup\$
1
\$\begingroup\$

The splitting isn't trivial. Therefore I wouldn't rely on scan() and regular expressions. I would first compute the count of sms and then use a text stream via StringIO and the pull the text.

Here is the first trial of the implementation.

def send_sms_message(text, to, from)
  count = split_count(text)
  if count == 1
    deliver_message_via_carrier(text, to, from)
  else
    text_stream = StringIO.new(text)
    1.upto(count).each do |n|
      suffix = " - Part #{n} of #{count}"
      content = text_stream.read(160 - suffix.length) + suffix
      deliver_message_via_carrier(content, to, from)
    end
  end
end

def split_count(text)
  rest_length = text.length
  count = 0
  while rest_length > 0
    count += 1
    case count
    when        1 then rest_length -= 160
    when        2 then rest_length -= 160 - 14 - 14
    when     3..9 then rest_length -= 160 - 14
    when       10 then rest_length -= 160 - 16 - 9
    when   11..99 then rest_length -= 160 - 16
    when      100 then rest_length -= 160 - 18 - 99
    when 101..999 then rest_length -= 160 - 18
    end
  end
  count
end

The complex part is the computation of the split_count. The trial version has room for improvements.

  • You could compute the magic numbers by computations. For example the length of the suffix could be calculated with 14 + 2 * Math.log10(count).floor. Maybe the whole case expression could be a function which returns the decrease value for rest_length.
  • You could use memoization/caching for performance.
  • You could take look at Dynamic Programming which was my inspiration for the implementation.

Update

I removed the magic numbers by constants and functions. Here is my final solution without any optimizations.

SMS_LENGTH = 160
SUFFIX_TEMPLATE = ' - Part %d of %d'

def send_sms_message(text, to, from)
  count = split_count(text)
  if count == 1
    deliver_message_via_carrier(text, to, from)
  else
    text_stream = StringIO.new(text)
    1.upto(count).each do |i|
      suffix = render_suffix(i, count)
      content = text_stream.read(SMS_LENGTH - suffix.length) + suffix
      deliver_message_via_carrier(content, to, from)
    end
  end
end

def split_count(text)
  rest_length = text.length
  count = 0
  while rest_length > 0
    count += 1
    rest_length -= payload_size(count)
  end
  count
end

def render_suffix(i, count)
  format(SUFFIX_TEMPLATE, i, count)
end

def payload_size(nth)
  case nth
  when 1 then SMS_LENGTH
  when 2 then SMS_LENGTH - 2 * render_suffix(nth, nth).size
  else SMS_LENGTH - render_suffix(nth, nth).length - (is_power_of_10?(nth) ? nth - 1 : 0)
  end
end

def is_power_of_10?(n)
  Math.log10(n) % 1 == 0
end
\$\endgroup\$
0
\$\begingroup\$

Sometimes in software you should alter the requirements to simplify the life of everyone (including the business people presumably asking for this to be done).

I would push back against the requirement to pre-calculate the total number of messages. Rather than pre-calculating utilize an end of message marker instead (the text END should suffice).

I used the solution below to simulate sending an entire book via SMS, and it required fewer than 700 messages total.


For example, if you changed your message format to exclude the "... of X" portion, and did the following instead:

<msg part    > - Part 1
<msg part    > - Part 2
...
...
...
<msg part> - Part 12657
END

... then your implementation becomes much more straightforward (though even this sample implementation should be cleaned up):

MULTIPART_SUFFIX = ' - Part '

def deliver_message_via_carrier(text, to, from)
  puts text
end

def send_sms_message(text, to, from)
  chars = text.chars
  counter = 0

  if text.length <= 160
    deliver_message_via_carrier(text, to, from)
  else
    while chars.length > 0
      counter += 1
      suffix_text = "#{MULTIPART_SUFFIX}#{counter}"
      msg_part = chars.shift(160 - suffix_text.length).join('')

      deliver_message_via_carrier("#{msg_part}#{suffix_text}", to, from)
    end
    deliver_message_via_carrier('END', to, from)
  end
end

send_sms_message(text, :sender, :receiver)

...and you get the following benefits:

  • Can send messases of arbitrary length (though let's be honest - you'll probably never send more than 1,000,000 messages, so supporting messages of length 10^70 -- many, many times the total amount of data that all of humanity has ever stored -- is highly dubious).
  • Your message suffix is shorter, so you wind up saving many characters per message, resulting in many fewer total messages for any message you're likely to actually send.
  • This alternative format allows you to easily make every partial message exactly 160 characters in length, which is more efficient on a per-message basis.
  • Your code's logic is much simpler, and therefore easier to maintain.
  • Messages are still able to be sorted on the receiving end if necessary (i.e. if they are received out-of-order for some reason), because they are individually numbered.
\$\endgroup\$

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.