2
\$\begingroup\$

I know that Facebook has an algorithm to filter out posts that may be irreverent, but I'm building a very simple yet functional algorithm to improve on it.

It's getting complicated to maintain, and I want to know how to organize the code in one specific way, have patterns, use shortcuts etc...

Please don't hesitate to criticize this code heavily.

#include <iostream>
#include <string>
#include <sstream>
#include <algorithm>
#include <iterator>
#include <vector>
#include <limits>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <math.h>

// TO DO: Add ignoring case and white space
// Make it so that the user doesn't have to enter each post from FB to this script to see if it's okay or not
// Add GUI to this script (wxWidgets?)

using namespace std;

vector<string> explode(const string& str, const char& ch) {
    string next = "";
    vector<string> result;

    // For each character in the string
    for (string::const_iterator it = str.begin(); it != str.end(); it++) {
        // If we've hit the terminal character
        if (*it == ch) {
            // If we have some characters accumulated
            if (next.length() > 0) {
                // Add them to the result vector
                result.push_back(next);
                next = "";
            }
        } else {
            // Accumulate the next character into the sequence
            next += *it;
        }
    }

    return result;
}


bool overuse_of_caps(int percentage, string this_message) {
    string ac[] = {"A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"};
    string otherChars[] = {" ", ",", ".", "-"};
    float x = 0; // Counter
    float y = 1; // Inverse Counter

    for (int i = 0; i < this_message.size(); i++) {
        for (int ii = 0; ii < 26; ii++) {
            for (int iii = 0; iii < 4; iii++) {
                if (!(this_message[i] == ' ')) {

                    stringstream ss;
                    string s;
                    char c = this_message[i];
                    ss << c;
                    ss >> s;

                    if (s == ac[ii]) {
                        x++;
                    }
                }
                stringstream ss;
                    string s;
                    char c = this_message[i];
                    ss << c;
                    ss >> s;
                if (s == otherChars[iii]) {
                    y++;
                }
            }
        }
    }



    if ( floor ( (x / ( ( this_message.size() ) - y) ) * 100) > percentage) {
        return true;

        } else {

        return false;

        }



}


int main (int argc, const char * argv[]) {


    // INITIALIZE
    bool is_true         = false;
    bool master_switch   = false;
    bool is_true_II      = false;
    bool true_III        = false;
    string alphabetLower = "abcdefghijklmnopqrstuvwxyz";
    string alphabetUpper = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    int questionMarkCounter = 0;
    int exclemMarkCounter = 0;
    string conditionals[] = {"like this", "my status", "like this status", "inbox", "bad mood", "r.i.p", "rip", "rest in peace", "cool beans", "im bored", "S I N G L E or N O T", "Single or not", "[ ]", "[", "]", "[]", "[x]", "shut up", "what the", ":(", ":'(", ":*", "((", "war", "MW", "MW2", "attack", "kill", "one person in this world", "iloveyou", "i love you", "idiot", "brotips", "tips", "ohgodlol", "Ill Tell You", "What I Think Of You", ".)", "if beauty were inches, you'd go on for miles.", "profile", "cracked", "drugs", "weed", "aint", "ain't", "Pokemon", "Lyrics", "jonas brothers", ":#", "heartbreaking", "</3", "<3", "SAVE", "survey", "purchase", "butt", "mediafire", "text it", "phone broken", "phones broke", "phone not working", "dont text", "don't text", "angrey", "sorrow", "on Twitter", "arrested", "watching", "farmvile", "21 questions", "profile view", "poop", "profile counter", "gurl", "girl", "hating", "haters", "hatin", "macdonalds", "mac donalds", "mcdonalds", "kiss", "LMFAO", "live webcam", "facebook video", "facebook", "PLEASE SHARE, THANKS!", "repost", "Barbie", "homework", "depress", "dog", "cat", "female", "thb", "tbh", "niggle", "ngl", "ho", "hoe", "hate", "dont", "hate", "dislike", "recreate", "disallow", "forget", "relationship", "relationship", "broke", "break", "respect", "forever", "never", "live", "life", "inbox", "status", "bored", "wtf", "wft", "wtfz", "hahahah", "hahha", "hahahahah", "hahaaah", "frigger", "effing", "eff", "bed"};


    // This is where the first test string is initialized.
    std::string this_message_RAW = "like my status because its awesome.";

    // Explode the string into words.
    std::vector<std::string> this_message = explode(this_message_RAW, ' ');



    // TEST ONE: Detect if the message is shorter or equal to 3 letters.
    if (this_message.size() <= 3) {
        master_switch = true;
    }


    // TEST TWO: Keyword matching
    for (int i = 0; i < this_message.size(); i++) {
        string this_item = this_message[i];


        for (int ii = 0; ii < 130; ii++) { // 130 needs to be updated to however many items are in the list.

            // Messing around with conditionals goes here.
            if (this_item == conditionals[ii]) {
                printf("A MATCH WAS FOUND.");
                master_switch = true;
            }

        }

    }       

    // TEST THREE: Check if the first item starts with a number.
    if (isdigit(this_message_RAW[0])) {
        bool FAILED_TEST = true;
        printf("Test three failed!");
        master_switch = true;
    }


    // TEST FOUR: Detect excessive use of question marks or exclementation marks.
    for (int i = 0; i < this_message_RAW.size(); i++) {
        if (this_message_RAW[i] == '?') {
            questionMarkCounter++;
        }
        if (this_message_RAW[i] == '!') {
            exclemMarkCounter++;
        }
    }

    if ((questionMarkCounter > 3)||(exclemMarkCounter > 3)) {
        printf("The question mark or exclem test failed.");
        master_switch = true;
    }


    // TEST FIVE: Overuse_of_caps
    if (overuse_of_caps(80, this_message_RAW)) {
        printf("Overuse of caps DETECTED!  Test failed.");
        master_switch = true;
    }


    // CONCLUSION
    if (is_true||is_true_II||true_III||master_switch) {
    printf("A test failed.  Not safe for public use.");master_switch = true;
        return false;}
    else{
        return true;
    }


    if (master_switch = true) {

        printf("One or more of the tests failed.  This means that this status is not good.");

    } else {

        printf("Passed!  The status is approved, and is okay.");

    }


    return 0;



}
\$\endgroup\$
8
\$\begingroup\$

Use the C++ versions of the header file.
This way everything is put in the std namespace.

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>

// Change to:

#include <cstdio>
#include <cstdlib>
#include <cctype>

Prefer not to import the whole std namespace into global namespace.

using namespace std;

It is not that hard to use the full name.

std::vector

Alternatively you can selectively import the bits you want with

using std::vector;

Not this is bound the current scope this you can limit its affect to just a function.

There is an easier way to write explode():
This example breaks the string into words using white space as the delimiter.
If you want to use another delimiter that is relatively easy; just use a type that reads words by delimiter when using the operator>>.

std::vector<std::string> explode(std::string const& str)
{
    std::stringstream         stream(str);
    std::vector<std::string>  result;

    std::copy(std::istream_iterator<std::string>(stream),
              std::istream_iterator<std::string>(),
              std::back_inserter(result)
             );
    return result;
}

There is no need to have a triple nested loop:

for (int i = 0; i < this_message.size(); i++) {
    for (int ii = 0; ii < 26; ii++) {
        for (int iii = 0; iii < 4; iii++) {

Basically you are testing each character to see if it is uppercase or a special. This is much easier by doing an explicit test on each character.

This is a very bad way of extracting a single character into a string.

                stringstream ss;
                string s;
                char c = this_message[i];
                ss << c;
                ss >> s;

The equivalent of the above code is:

                this_message.substr(i,1);

But what you really want is:

                char c = this_message[i];

The test for uppercase letters would be much easier written as:

     isupper(this_message[i])

The test for a specific character in a set you can use:

     strchr(" ,.-", this_message[i]) != NULL

Using these two we could reduce your code too:

bool overuse_of_caps(int percentage, string this_message)
{
    float x = 0; // Counter
    float y = 1; // Inverse Counter

    for (int i = 0; i < this_message.size(); i++)
    {
        if (isupper(this_message[ii]))
        {
            x++;
        }
        if (strchr(" ,.-", this_message[i]) != NULL)
        {
            y++;
        }
    }

    return ( floor ( (x / ( ( this_message.size() ) - y) ) * 100) > percentage);
}

You don't need to write a manual search the standard already has one:

std::find(iterator, iterator, thingToFind)

So this:

    for (int ii = 0; ii < 130; ii++) { // 130 needs to be updated to however many items are in the list.

        // Messing around with conditionals goes here.
        if (this_item == conditionals[ii]) {
            printf("A MATCH WAS FOUND.");
            master_switch = true;
        }

Can be replaced with:

master_switch = std::find(conditionals.begin(), conditionals.end(), this_item) != conditionals.end();
\$\endgroup\$
6
\$\begingroup\$

I haven't really tried to understand your code but I have a few remarks :

  • There's a much better way than using an array to get the i-th letter of the alphabet.

    string ac[] = {"A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"};
    // ...
    if (s == ac[ii])
    

    Can be rewritten /

        if (s == ii+'A')
    
  • You can improve readibility of your code by writing :

    if (this_message[i] != ' ')
    

    instead of:

    if (!(this_message[i] == ' '))
    
  • Use else between 2 conditions that cannot be true in the same time. By doing do, you won't evaluate the second condition when the first one is true (it's slighty better from a performance point of view but it's also much easier for the code-reader to follow the flow). Instead of:

    if (this_message_RAW[i] == '?') { foo(); }
    if (this_message_RAW[i] == '!') { bar(); }
    

    Write

    if (this_message_RAW[i] == '?') { foo(); }
    else if (this_message_RAW[i] == '!') { bar(); }
    
  • There's no point in having this :

    if ( floor ( (x / ( ( this_message.size() ) - y) ) * 100) > percentage)
    { return true; }
    else 
    { return false; }
    

    When you can write:

    return ( floor ( (x / ( ( this_message.size() ) - y) ) * 100) > percentage);
    
  • Please take into account your own comments before asking for code review. You can easily use the length of your list instead of a hardcoded value in

    for (int ii = 0; ii < 130; ii++) { // 130 needs to be updated to however many items are in the list.
    
  • Please take into account the compiler warnings before asking for code review. I'm pretty sure that the code after

    if (is_true||is_true_II||true_III||master_switch) {
            printf("A test failed. Not safe for public use.");master_switch = true;
            return false;}
    else{
            return true;
    }
    

    will never be reached and that should be said by the compiler.

  • I don't get the point of having bool is_true = FALSE;. But there are many things to be said here:

    • This could be const
    • You can use false in lowercase
    • This variable is useless (this might also be detected by your compiler)
    • The bool name is really really confusing

Same remarks apply to most of the other variables.

\$\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.