1
\$\begingroup\$

Assume we have an string that contains the character 'c'. This character can be located anywhere within the string. The exercise is to remove 'c', and the characters that, if they exist, are on its immediate left or right. I have devised an algorithm, assuming that 'c' = '*'. Its flowchart can be found here and the code is as follows:

In C#:

class Program
{
    static void Main(string[] args)
    {
        string input = Console.ReadLine();

        Console.WriteLine(RemoveAsterix(input));
        Console.ReadKey();
    }

    static string RemoveAsterix(string myString)
    {
        if (!(myString.Contains('*')))
        {
            throw new Exception("You need an asterix!");
        }

        List<char> myStringSplit = myString.ToCharArray().ToList();

        if (myStringSplit[0] == '*')
        {
            myStringSplit.RemoveRange(0, 2);

        }

        if (myStringSplit[myStringSplit.Count - 1] == '*')
        {
            myStringSplit.RemoveRange(myStringSplit.Count - 2, 2);

        }

        for (int i = 1; i < myStringSplit.Count - 1; i++)
        {
            if (myStringSplit[i] == '*')
            {
                myStringSplit.RemoveRange(i - 1, 3);
                break;
            }
        }

        return new string(myStringSplit.ToArray());

    }
}

And in Python:

def RemoveAsterix(myString):
    myStringList = list(myString)

    if '*' not in myString:
        raise ValueError("String must contain asterix")

    if myStringList[0] == '*':
        del myStringList[0:2]


    if myStringList[-1] == '*':
        del myStringList[-2:]


    for i in range(1, len(myStringList) - 2):
         if myStringList[i] == '*':
             del myStringList[i-1:i+2]
             break;


    return ''.join(myStringList)


def Main():
    print(RemoveAsterix(input("Enter a string: ")))

Main()

Please also review my algorithm.

\$\endgroup\$
  • 3
    \$\begingroup\$ Asterix != Asterisk :) \$\endgroup\$ – Martin R Jul 9 '16 at 8:21
  • \$\begingroup\$ @MartinR: Huh. Saved me from future embarrassment. \$\endgroup\$ – CodesInTheValley Jul 9 '16 at 8:26
  • 1
    \$\begingroup\$ You have to add the expected input types and intended behaviours for multiple asterisk in succession ; with the current code for example "***" as argument will crash \$\endgroup\$ – Sehnsucht Jul 9 '16 at 8:55
  • \$\begingroup\$ @Sehnsucht: Noted. \$\endgroup\$ – CodesInTheValley Jul 9 '16 at 9:34
  • 1
    \$\begingroup\$ What is the output for "*abc*defg*hilm*" ? \$\endgroup\$ – Caridorc Jul 9 '16 at 14:01
3
\$\begingroup\$

Generalise

Having a method RemoveAsterix isn't a scaleable approach to solving the general case. A better method might be:

static string RemoveLettersSurroundingCharacter(string sourceString, char charToRemove)

Passing in the character to remove also helps you to remove a lot of the hard coded * references in your code.

Variable names

Avoid names that start with 'my'. It reads very much like example code that isn't meant for production, although some people also use it as a naming convention when referring to member variables of a class (which you aren't).

Algorithm

As has already been said in the comments, your algorithm is quite naive. It works for simple cases, but doesn't seem to have been tested for many edge cases that you might expect to encounter.

**1         => 1
***         => Crash
1234***567  => 123*567
1234**      => 1234
ab*cd*ef    => ad*ef

Your algorithm will remove any leading *, any trailing * and the first instance of * outside of these positions. This leaves its processing in a bit of a weird state of completeness. If you only want to process a single *, then you should check to ensure that the string only contains a single * and error if it doesn't. If you want to support removal of any number of *, then you need to think about what to do when you encounter two * next to each other. Does the second one get ignored (because it has been removed when the first is encountered) or does it extend the removal. This is the difference between: 12**34 turning into 134 or 14.

Use specific exceptions where available

You want to throw the most specific exception that you can. Generally speaking, you should avoid throwing and catching base exceptions. So, rather than:

throw new Exception("You need an asterix!");

It would be better to:

throw new ArgumentException("You need an asterix!", "myString");
\$\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.