Tokenize arithmetic expression in C This program tokenizes an arithmetic expression specified in infix notation. It is one of my personal projects and I would love to receive expert advice.
After compiling, you can run the program with one command-line argument:
$ tokenize <arith_expr>

This is an example of running the program:
$ tokenize "3^2 + 4.5 * (-2 - 1)"

The tokenizer currently supports these operators + - * / ^.
#include <ctype.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* Can store an operator or an operand */
struct token {
    char operator;
    float operand;
};

/* Check if given token is an operator */
bool is_operator(char token)
{
    if (token == '+' ||
        token == '-' ||
        token == '*' ||
        token == '/' ||
        token == '^')
        return true;
    else
        return false;
}

/* Remove all spaces from string */
void rmspaces(char *str)
{
    const char *dup = str;
    do {
        while (isspace(*dup))
            ++dup;
    } while (*str++ = *dup++);
}

/* Return first token of given arithmetic expression */
struct token *tokenize(char **expr)
{
    static bool op_allowed = false;
    struct token *token = malloc(sizeof *token);
    if (!token) {
        printf("Error: memory allocation failed");
        exit(EXIT_FAILURE);
    }

    if (op_allowed && is_operator(*expr[0])) {
        token->operator = *expr[0];
        ++(*expr);
        op_allowed = false;
    } else if (!op_allowed && *expr[0] == '(') {
        token->operator = *expr[0];
        ++(*expr);
    } else if (op_allowed && *expr[0] == ')') {
        token->operator = *expr[0];
        ++(*expr);
    } else {
        char *rest;
        token->operand = strtof(*expr, &rest);
        token->operator = '\0';
        if (*expr == rest) {
            printf("Invalid expression\n");
            exit(EXIT_FAILURE);
        }
        strcpy(*expr, rest);
        op_allowed = true;
    }

    return token;
}

/* Handle command line arguments */
int main(int argc, char *argv[])
{
    if (argc != 2) {
        printf("Usage:   %s <arith_expr>\n"
            "Example: %s \"5 2 3 * +\"\n",
            argv[0], argv[0]);
        exit(EXIT_FAILURE);
    }

    char *expr = argv[1];
    rmspaces(expr);
    struct token *token;
    while (expr[0] != '\0') {
        token = tokenize(&expr);
        if (token->operator == '\0') // token is operand
            printf("%f\n", token->operand);
        else
            printf("%c\n", token->operator);
    }

    return EXIT_SUCCESS;
}

Your is_operator function can be rewritten as
return token == '+' || token == '-' || token == '*' || token == '/' || token == '^';

Much cleaner, in my opinion.

The program has a memory leak. Your tokenize function allocates a struct token on the heap, but the object is never freed.
Better yet, instead of returning a struct token*, just return a struct token. The struct certainly isn't large enough to warrant an allocation on the heap.

Instead of checking whether token is an operand by comparing operator to 0, use an enum.
typedef enum {
       TokenType_Operator,
       TokenType_Operand
} token_type;

Then your token struct can consist of the token_type enum, and an union.
struct token
{
   token_type type;
   union {
       char operator;
       float operand;
   } data;
};

If you're using C11, you don't even need to name the union anything.

As a next step, try to implement a tokenizer that tokenizes in one pass. Right now, your tokenizer 1) removes whitespace (which involves a linear pass) and 2) copies of the rest of the string after the tokenizing an operand into the original buffer. If you have a large buffer, this could become a bottleneck.
A common way to implement a tokenizer is using a DFA (deterministic finite automata). In fact, a DFA is a starting point for most handwritten lexers.


Count reoccurring elements in multiple lists in Python I have a number of names divided into several lists. I am printing all the names which occur in more than one list, sorted by the number of occurrences.

What is a better/easier/more pythonic way of doing this? I feel like my solution is overly complicated.

import numpy as np

list_1 = ['John Cleese', 'Terry Gilliam']
list_2 = ['Eric Idle', 'Terry Jones', 'Michael Palin']
list_3 = ['Graham Chapman', 'Sir Lancelot the Brave', 'Terry Jones']
list_4 = ['Arthur, King of the Britons', 'Terry Jones', 'John Cleese']
list_5 = ['Michael Palin', 'Sir Robin the Not-Quite-So-Brave-as-Sir-Lancelot']

all = sorted(np.unique(list_1+list_2+list_3+list_4+list_5))

def in_how_many(name):
    result = 0
    if name in list_1:
        result += 1
    if name in list_2:
        result += 1
    if name in list_3:
        result += 1
    if name in list_4:
        result += 1
    if name in list_5:
        result += 1

    return result

names_list = []

for name in all:
    if in_how_many(name) > 1:
        name_dict = {'name': name, 'value': in_how_many(name)}
        names_list.append(name_dict)

for person in sorted(names_list, key=lambda k: k['value'], reverse=True):
    print '\'%s\' is in %s lists' % (person['name'], person['value'])

This prints:

'Terry Jones' is in 3 lists
'John Cleese' is in 2 lists
'Michael Palin' is in 2 lists

What you want is collections.Counter, and the Counter.most_common method.

A naive implementation could be:

import collections

lists = [list_1, list_2, list_3, list_4, list_5]
counter = collection.Counter(sum(lists))

for name, amount in counter.most_common():
    print('\'%s\' is in %s lists' % (name, amount))

This however will mean if list_1 is ['a'] * 5 it will say 'a' is in five lists when this isn't true. A dictionary comprehension is all that's needed for this building a dictionary from the lists, setting the value to 1. And you can take advantage of the fact collection.Counter has addition defined.

counter = sum(
    collection.Counter({name: 1 for name in names})
    for names in lists
)

Without using collections.Counter I would advise you use set. Firstly it removes the need for using np.unique and emphasis the fact that sorted is redundant, as sets are unordered.

It also means that making all is simple and reduces name in list_1 from an \$O(n)\$ operation to an \$O(1)\$ operation. Leading to faster code.

As shown earlier it's easier to work with lists, rather than a list of variable names and so lists is defined the same as above.

sets = [set(names) for names in lists]
all = sum(sets)

def in_how_many(value, sets):
    return sum(
        value in set_
        for set_ in sets
    )

No matter which of the above two solutions you use you still have a couple of other problems:

Don't put code in global scope, have a main function. This makes it harder to do bad things or mess up.
You should use if __name__ == '__main__' to only allow the code in main to run if it's the 'main' file.
In relation to the above two, notice that I pass sets to in_how_many. This is because you shouldn't rely on global scope as it makes reusing the same code harder to do.
Modulo formatting was deprecated for a while, because it's generally worse than str.format and is more susceptible to errors. I suggest using str.format or upgrading to Python 3.7 to take advantage of f-strings.
Import print_function form __future__ to make print a function. This makes it easier to upgrade to Python 3.

from __future__ import print_function
import collections
import functools
import operator

def main(lists):
    counter = functools.reduce(
        operator.add,
        (
            collections.Counter(set(names))
            for names in lists
        )
    )

    for name, amount in counter.most_common():
        print('{name!r} is in {amount} lists'.format(name=name, amount=amount))
        # print(f'{name!r} is in {amount} lists')  # Python 3.7 f-string.

if __name__ == '__main__':
    main(...)


Loading data from a web API to be displayed in a tableview In my object Dish xxx.Dish, I want to access the Choice class    price and name to display . dish data load from web API and I tested data loaded success full and put the data to the object dish and it return the object list to viewcontroller to load tableview.

Output of printed console

  Optional([xxx.Dish, xxx.Dish])

and in the dish class before append optionList?.append(_obj)

  xxx.DishOption

I am new to swift and is it right way to implement? Please suggest me?

class Dish {
        let dishId : String
        var optionList : [DishOption]?

        init?(fromAPIResponse resposne : Dictionary<String,AnyObject>) {

            guard let dishId = resposne["dishId"] as? String else {
                return nil

            }
            self.dishId = dishId
            if let objs =  resposne["options"] as?  [[String: AnyObject]]{
                optionList = [DishOption]()
                for obj in objs {
                    if  let _obj = DishOption(fromAPIResponse: obj){

                        optionList?.append(_obj)

                    }
                }
            }
        }

//another file

   class DishOption {

                let optionId : String

                var choiceList : [Choice]?

                init?(fromAPIResponse resposne : Dictionary<String,AnyObject>) {

                    guard let  optionId = resposne["optionId"] as? String else {
                        return nil
                    }

                    self.optionId = optionId

                    if let objs =  resposne["choices"] as?  [[String: AnyObject]]{
                        choiceList = [Choice]()
                        for obj in objs {
                            if  let _obj = Choice(fromAPIResponse: obj){

                                choiceList?.append(_obj)

                            }
                        }
                    }
                }
            }

 class Choice{
           let choiceId : String
                    let name : String
                    let price : String

                    init?(fromAPIResponse resposne : Dictionary<String,AnyObject>) {
                        guard let choiceId = resposne["choiceId"] as? String ,
                            let name = resposne["name"]  as? String,
                            let price = resposne["price"] as? String else {

                                return nil
                        }
                        self.choiceId = choiceId
                        self.name = name
                        self.price = price

                    }

            }

Usage:

     var  dishMenuList = [Dish]()
            guard let objs = json["menu_list"] as? [[String : AnyObject]] else {
                return
            }

            for obj in objs {

                if let _obj = Dish(fromAPIResponse: obj){

                 print(_obj.optionList) //always print nil

                if let options = _obj.optionList {

                    for data in options {

                        print(data.displayAsButton)

                    }
                   }
                   dishMenuList.append(_obj)
                }

            }

1) i would not use inner classes.

2) i would extract the data extraction part from the init to a other class / helper - a class should only have the data structure and not how to convert it

3) typo resposne => response


Exporting an object to a ZIP file I have my own type of object, which implements Serializable. Here I have one called "extendedFormat" (not a terribly helpful name, yes, I know). I'm writing it out to a file like so:

try {
    // is this the best way to do this????????
    // this seems like too many streams perhaps.
    // like maybe this is now a river
    File file = new File("C:/temp/tempExport.bef");
    file.createNewFile();
    FileOutputStream fileStream = new FileOutputStream(file);
    BufferedOutputStream bufferStream = new BufferedOutputStream(fileStream);
    ZipOutputStream zipStream = new ZipOutputStream(bufferStream);
    zipStream.putNextEntry(new ZipEntry("bef"));
    ObjectOutputStream objectStream = new ObjectOutputStream(zipStream);
    objectStream.writeObject(extendedFormat);
    zipStream.closeEntry();
    objectStream.close();
    zipStream.close();
    bufferStream.close();
    fileStream.close();
} catch (FileNotFoundException e) {
    // TODO Auto-generated catch block
    e.printStackTrace();
} catch (IOException e) {
    // TODO Auto-generated catch block
    e.printStackTrace();
}

Is this the best way to do this? It seems a little over complicated but I want to save as much time writing it out and also space in terms of how big the file is that I write out.

I know this may seem like a lot for one object, but it can hold an entire table with possibly hundreds of columns and thousands of rows, so efficiency is important.

You could use try-with-resources  to efficiently close the streams, this also reduces line count and readability.

IOUtils provides a tested utility to copy between input and output streams, add commons-compress to you classpath, for example using gradle

 compile group: 'org.apache.commons', name: 'commons-compress',version: '1.14'

I simply extended the code you provided. I believe you will get the idea for reading back the zip file.

  try {

            File file = new File("C:/temp/tempExport.bef");
            file.createNewFile();

//Use try with resources to handle closing streams

            try (ZipOutputStream zipStream = new ZipOutputStream(new BufferedOutputStream(new FileOutputStream(file)))) {
                zipStream.putNextEntry(new ZipEntry("bef"));
                ObjectOutputStream objectStream = new ObjectOutputStream(zipStream);
                objectStream.writeObject(extendedFormat);
            }

         //to read the file you need a temp buffer or file to decompress the archive
            File temp = Files.createTempFile("", "").toFile();

            try (ZipInputStream zis = new ZipInputStream(new BufferedInputStream(new FileInputStream(file)));
                 BufferedOutputStream bos = new BufferedOutputStream(new FileOutputStream(temp));
                 BufferedInputStream bis = new BufferedInputStream(new FileInputStream(temp))) {
                zis.getNextEntry();
                IOUtils.copy(zis, bos);
                bos.close();
                ObjectInputStream ois = new ObjectInputStream(bis);
                ExtendedFormat extendedFormat= (ExtendedFormat) ois.readObject();
            }

            temp.delete();

        } catch (FileNotFoundException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (ClassNotFoundException e) {
            e.printStackTrace();
        }


Middleware Django statistics I have this middleware to log the number of queries, the time spent on each view and some errors and tracebacks.

This will be called on each request, so I had to put a 75ms timeout. I don't like it, but I feel like using async request for this is unnecessary, I don't care about the host response.

What do you think? Can I improve this in any way?

import sys
import traceback
import os
import json
import time
import requests
from django.conf import settings

CUSTOM_LOG_AGENT_HOST = 'http://localhost:9875'
CUSTOM_LOG_AGENT_HEADERS = {}
CUSTOM_LOG_AGENT_TIMEOUT = 0.0075

class SimpleMiddleware(object):
    def __init__(self, get_response):
        self.get_response = get_response
        # One-time configuration and initialization.

    def __call__(self, request):
        self.customLogAgent = {}

        request.start_time = time.time()
        # Code to be executed for each request before
        # the view (and later middleware) are called.
        response = self.get_response(request)
         # Code to be executed for each request/response after
        # the view is called.

        total = time.time() - request.start_time
        self.customLogAgent['queries'] = connection.queries
        self.customLogAgent['time'] = int(total * 1000)
        try:
            url = getattr(settings, 'CUSTOM_LOG_AGENT_HOST', CUSTOM_LOG_AGENT_HOST)
            headers = getattr(settings, 'CUSTOM_LOG_AGENT_HEADERS', CUSTOM_LOG_AGENT_HEADERS)
            timeout = getattr(settings, 'CUSTOM_LOG_AGENT_TIMEOUT', CUSTOM_LOG_AGENT_TIMEOUT)

            requests.post(url, headers=headers, json=self.customLogAgent, timeout=timeout)
        except requests.exceptions.RequestException as e:
            print(e)
        print(json.dumps(self.customLogAgent, indent=4))

        return response

    def process_view(self, request, view_func, view_args, view_kwargs):
        self.customLogAgent['view_func'] = view_func.__name__
        self.customLogAgent['view_kwargs'] = view_kwargs
        self.customLogAgent['url'] = request.get_full_path()

    def process_exception(self, request, exception):
        ltype, lvalue, ltb = sys.exc_info()
        error = {}
        error['type'] = ltype.__name__
        error['message'] = str(lvalue)
        error['line'] = ltb.tb_lineno
        error['traceback'] = ''.join(traceback.format_tb(ltb))
        self.customLogAgent['error'] = error
        return None

I believe that performance is extremely important on websites.
I would consider any delay at all due to requests to a logging server unacceptable.
I think you should move the logging call to a background thread,
and return the response to the user as quickly as possible.

As usual, it's probably a good idea to put a cap on the maximum number of background threads. It could be even just one thread doing all the logging tasks in sequence.

Other than this, the code looks fine to me.
It's simple and not easy to pick on.


Converting stacktrace to string I am just sharing the method that i am using to convert stacktrace to string. This stacktrace is then being used in AWS Lambda to log. There are two things i am mainly concerned about

private String convertStackTrace(Throwable throwable){
    StringWriter stringWriter = new StringWriter();
    PrintWriter printWriter = new PrintWriter(stringWriter);
    String stackTrace;

    try {
        throwable.printStackTrace(printWriter);
        stackTrace = stringWriter.toString();
    }
    catch(Exception e){
        logSelf("Error converting exception. Simple exception message will be logged");
        stackTrace = throwable.getMessage();
    } finally {
        try {
            stringWriter.flush();
            stringWriter.close();
            printWriter.flush();
            printWriter.close();
        } catch (Exception e){
            logSelf("Error closing writers");
        }
    }

    return stackTrace;
}

Here is logSelf method

private void logSelf(String error){
    lambdaLogger.log(formatMessage(
            this.getClass().getCanonicalName(), LOG_LEVEL_ERROR, error, null)
    );
}

Shall i be opening/closing those printwriters everytime an error is logged?
Is it correct way to convert stacktrace to string?

Yes, you should be opening/closing the printwriters each time.

Yes, conceptually it's a decent way to convert the stack trace..... but... there are concerns, and is there a better way?

The most significant concern is that, if there's a problem writing to the PrintWriter, the call throwable.printStackTrace(printWriter); will fail, and the method will return a default value. This is not ideal.

The reality, though, is that those methods can never fail because the IO is to a StringWriter, and there's no actual IO component. A failure there would be..... inconceivable. None of your exception handling can really happen... it's just not going to fail (the "Titanic Logger").

The issue is that your code is checking for impossible conditions in a way that's overkill.

Still, using some Java 7 semantics, you can use try-with-resource functionality, and get a much more concise method:

private static String convertStackTrace(Throwable throwable) {
    try (StringWriter sw = new StringWriter();
             PrintWriter pw = new PrintWriter(sw)) {

        throwable.printStackTrace(pw);

        return sw.toString();

    } catch (IOException ioe) {
        // can never really happen..... convert to unchecked exception
        throw new IllegalStateException(ioe);
    }
}

Note that the method is now static, and it does not need the finally section to clean up.

You can see this running in ideone: https://ideone.com/rKj9mT


Linq in foreach with c# Im running a photocontest and making a list of winners with one winner each day. My loop is running very slow. It seams to be the toList() that is time consuming. Any suggestions on how to make it run faster?

foreach (var date in dates)
        {

            var voteList = _images.Select(image => new ImageWinnerVotes
                                                   {
                                                       ID = image.ID,
                                                       Image = image.ImagePath,
                                                       Thumbnail = image.ThumbnailImagePath,
                                                       Title = image.Name,
                                                       Url = GetFriendlyUrl(image.ID),
                                                       Date = image.CreatedDate,
                                                       WinnerDateString = date.Date.ToString("dd/MM"),
                                                       WinnerDate = date.Date,
                                                       TotalVotes =
                                                           (votes.Where(vote => vote.ItemId == image.ID)).Count(),
                                                       Name = image.Fields["Name"].ToString(),
                                                       Votes =
                                                           (votes.Where(
                                                               vote =>
                                                               vote.ItemId == image.ID &&
                                                               vote.Date.DayOfYear == date.Date.DayOfYear)).Count()
                                                   }).ToList();

            voteList = voteList.OrderByDescending(v => v.Votes).ToList();

            var j = 0;
            var findingWinner = true;

            while(findingWinner)
            {
                var inWinnersList = false;
                if (winnersList.Count() != 0)
                {
                    if(!winnersList.Contains(voteList.ElementAt(j)))
                    {
                        winnersList.Add(voteList.ElementAt(j));
                        findingWinner = false;
                    }
                }
                else
                {
                    winnersList.Add(voteList.ElementAt(j));
                    findingWinner = false;
                }

                j++;
            }

        }

First up calls to .ToList() are not inherently slow - it's only when each element in the list is slow to compute that .ToList() appears slow.

So then, what makes the time to compute each element slow?

Well, you're querying votes twice in each query - and computing TotalVotes for each iteration of the foreach loop.

After the query you are then sorting all the candidates by daily votes and then searching through the candidates and picking the first candidate image that hasn't previously won. The search for the winner uses Count, Contains, and ElementAt - all of which can cause the list to be enumerated each time.

All of this is surrounded by a foreach loop to iterate through each date.

Lots of nested iterating - and most of which is unnecessary.

I've got a solution which I've timed to be 135 times faster.

Here it is:

var query =
    from image in _images
    join tvote in votes on image.ID equals tvote.ItemId into tvs
    let TotalVotes = tvs.Count()
    join vote in votes on image.ID equals vote.ItemId
    join date in dates on vote.Date.DayOfYear equals date.Date.DayOfYear
    group vote by new
    {
        image,
        date.Date,
        TotalVotes,
    } into dvs
    let Date = dvs.Key.Date
    let Image = dvs.Key.image
    let DailyVotes = dvs.Count()
    orderby DailyVotes descending
    orderby Date
    group new
    {
        Image = dvs.Key.image,
        dvs.Key.TotalVotes,
        DailyVotes = dvs.Count(),
    } by Date;

var winners = query.Aggregate(new ImageWinnerVotes[] { }, (iwvs, gs) =>
{
    var previousWinners = iwvs.Select(iwv => iwv.ID).ToArray();
    var winner = gs
        .Where(g => !previousWinners.Contains(g.Image.ID))
        .Select(g => new ImageWinnerVotes
        {
            ID = g.Image.ID,
            Image = g.Image.ImagePath,
            Thumbnail = g.Image.ThumbnailImagePath,
            Title = g.Image.Name,
            Url = GetFriendlyUrl(g.Image.ID),
            Date = g.Image.CreatedDate,
            WinnerDateString = gs.Key.ToString("dd/MM"),
            WinnerDate = gs.Key,
            TotalVotes = g.TotalVotes,
            Name = g.Image.Fields["Name"].ToString(),
            Votes = g.DailyVotes,
        })
        .FirstOrDefault();
    return winner == null ? iwvs.ToArray() : iwvs.Concat(new [] { winner, }).ToArray();
}).ToArray();

This is purely a LINQ solution. It's a good idea not to mix LINQ & non-LINQ.

Now, if you didn't have the requirement to eliminate previous winners then I have a solution that is twice as fast again:

var query =
    from image in _images
    join tvote in votes on image.ID equals tvote.ItemId into tvs
    let TotalVotes = tvs.Count()
    join vote in votes on image.ID equals vote.ItemId
    join date in dates on vote.Date.DayOfYear equals date.Date.DayOfYear
    group vote by new
    {
        image,
        date.Date,
        TotalVotes,
    } into dvs
    group new
    {
        Image = dvs.Key.image,
        dvs.Key.TotalVotes,
        DailyVotes = dvs.Count(),
        } by dvs.Key.Date into gxs
    let winners =
        from x in gxs
        orderby x.DailyVotes descending
        select x
    let winner = winners.First()
    let image = winner.Image
    let winnerDate = gxs.Key
    orderby winnerDate
    select new ImageWinnerVotes
    {
        ID = image.ID,
        Image = image.ImagePath,
        Thumbnail = image.ThumbnailImagePath,
        Title = image.Name,
        Url = GetFriendlyUrl(image.ID),
        Date = image.CreatedDate,
        WinnerDateString = winnerDate.ToString("dd/MM"),
        WinnerDate = winnerDate,
        TotalVotes = winner.TotalVotes,
        Name = image.Fields["Name"].ToString(),
        Votes = winner.DailyVotes,
    };

var winners = query.ToArray();

Let me know if these help and/or if you need more info.


Text Adventure Game in Python I'm currently working on a text adventure game for my first project. I am a beginner so I am seeking help with my code. Please comment any tips and general help regarding with making my code better, faster or more organised! I only started Python two weeks ago so any help would be greatly appreciated. I decided to only submit the code for the first room as I don't want to submit too much code at once.

health = 100
coins = 0
tutorial = True
gameStart = False
death  = False
spawn = True
livingRoom = False
Bathroom = False
Bedroom = False
Kitchen = False
wardrobeSpawn = False
spawnStart = True
telev = False

def countCoins():
    print
    print("You have: " + str(coins) + " coins")

while gameStart == True:
    if spawn == True:
        if spawnStart == True:
            print
            print(bcolors.FAIL + "You wake up in a room you don't
recognise, with no idea how you got there")
            print
            print(bcolors.ENDC + "You look around and see a wardrobe
and a door")
            spawnStart = False
        elif spawnStart == False:
            print
            spawnIn = raw_input("")
            spawnIn = spawnIn.lower()
            if spawnIn == "coins":
                countCoins()
                spawnStart = False
            elif spawnIn == "wardrobe":
                if wardrobeSpawn == False:
                    print
                    print "The wardrobe seems quite empty, but you do
find a coin in the far left corner, you put it in your pocket"
                    print
                    coins += 1
                    wardrobeSpawn = True
                    spawnStart = False
                elif wardrobeSpawn == True:
                    print "This wardrobe looks oddly familiar, you have
already explored it"
                    print
                    spawnStart = False
            elif spawnIn == "door":
                print
                print ("You walk out the door")
                print
                spawn = False
                livingRoom = True
            elif spawnIn == "look":
                print("You look around and see a wardrobe and a door")
                print
                spawnStart = False
            else:
                print("That is an unvalid command. Try again")
                print
                spawnStart = False

Bugs

In CountCoins:

print

... has no effect. The same applies to other parts of your code where you used this. If you meant to print a newline, use print() instead. You've also used print inconsistently, sometimes just leaving out the parentheses. Your tags include python-3.x, so you should fix that.

raw_input("")

... there's no such thing as raw_input in Python 3. I'm starting to think you've used a wrong tag.

Comparisons to True

Python automatically compares to True in if / elif / else / while statements. Therefore, you can shorten this:

while gameStart == True:

if spawn == True:

if spawnStart == True:

... to this:

while gameStart:

if spawn:

if spawnStart:

... what this also means is that instead of writing:

if wardrobeSpawn == False

... you can write:

if not wardrobeSpawn:

... which is shorter and more readable.

F-strings

Instead of doing:

print("You have: " + str(coins) + " coins")

You might want to use a so-called f-string:

print(f"You have {coins} coins")

This is easier for you and other developers.
f-strings are only available in Python 3.6 and later.

Functions

You seem to be using just one function. I suggest splitting the game up into several functions, perhaps like this:

def Level1():
    print("You wake up ...")

def Choice1()

def Choice2()

Styling

You should definitely read PEP8.
It states that you should, for example, use snake_case for variables and function names. Also, you should leave enough whitespace in between blocks of code to make it easier to read and manage. Really, take a look, it's a good read.


Formatting a possibly nil value as hours and minutes This might be a bit silly because there are like a thousand ways to do this, but I'm struggling to come up with one that I like. I feel like this should fit in one line, but I don't like re-using read_attribute(:length) lots and I don't like the below because it's not that clear.

# returns a human-readable string in the format Xh Ym
def length
  # length stored in seconds
  len = read_attribute(:length)
  "#{(len/60/60).floor}h #{len.modulo(60*60)/60}m" unless len.nil?
end

In one line

def length
  (len = read_attribute(:length)) && "#{(len/60/60).floor}h #{len.modulo(60*60)/60}m"
end

If len == nil return nil, if not - second operand


AES CTR mode encryption with HMAC I am trying to implement AES CTR encryption mode with HMAC authentication for messages.

It's encrypting and decrypting fine as long as the key length is 64 bytes, since AES key and HMAC key are being derived from this key.

Questions

Is it safe to append IV or nonce to the encrypted messages?
Is it safe to append HMAC digest to append to the messages?
Can you review it for best security coding practices?
I am using Pycryptodome

Code

import os
import zlib
from Crypto.Hash import HMAC
from Crypto.Hash import SHA256
from Crypto.Cipher import AES
from Crypto.Util import Counter
import zlib
def encrypt(full_key, plaintext):

    if len(full_key) != 64:
        raise Exception("FULL key length shall be equal to 64")
    key = full_key[:len(full_key) //2]

    # Use the last half as the HMAC key
    hmac_key = full_key[len(full_key) // 2:]

    if isinstance(plaintext, str):
        plaintext = plaintext.encode()

    compressed = zlib.compress(plaintext, 5)
    print (f"compressed plaintext {compressed}")

    # Choose a random, 16-byte IV.
    iv = os.urandom(16)
    # Convert the IV to a Python integer.
    iv_int = int(binascii.hexlify(iv), 16)
    # Create a new Counter object with IV = iv_int.
    ctr = Counter.new(128, initial_value=iv_int)
    # Create AES-CTR cipher.
    aes = AES.new(key, AES.MODE_CTR, counter=ctr)
    # Encrypt and return IV and ciphertext.
    ciphertext = aes.encrypt(compressed)

    hmac_obj = HMAC.new(hmac_key, compressed, SHA256)
    mac = hmac_obj.digest()

    return iv+ciphertext+mac

def decrypt(key, ciphertext):
    # Initialize counter for decryption. iv should be the same as the output of
    # encrypt().

    if len(full_key) != 64:
        raise Exception("FULL key length shall be equal to 64")

    key = full_key[:len(full_key) //2]

    # Use the last half as the HMAC key
    hmac_key = full_key[len(full_key) // 2:]

    mac_length = 32
    iv_length = 16
    iv = ciphertext[:16]
    mac = ciphertext[-mac_length:]

    _ciphertext = ciphertext[iv_length:-mac_length]

    iv_int = int(iv.hex(), 16)
    ctr = Counter.new(128, initial_value=iv_int)
    # Create AES-CTR cipher.
    aes = AES.new(key, AES.MODE_CTR, counter=ctr)

    ciphertext = aes.decrypt(_ciphertext)

    # Extract the MAC from the end of the file
    hmac_obj = HMAC.new(hmac_key, ciphertext, SHA256)
    computed_mac = hmac_obj.digest()

    if computed_mac != mac:
        raise Exception("Messege integrity violated")

    plaintext= zlib.decompress(ciphertext)

    # Decrypt and return the plaintext.

    return plaintext

I'm not a security expert, so my review will focus on general best practices first.

Code structure and style

1. Imports
You can group import that belong together, e.g

from Crypto.Hash import HMAC
from Crypto.Hash import SHA256

can be written as

from Crypto.Hash import HMAC, SHA256

The Official Style Guide for Python also recommends to place import from the standard library before any third party libraries. Apart from the double import of zlib you're already following this.

2. Documentation
You started to add some kind of documentation on decrypt, so encrypt should be treated the same. The officially recommended way is to provide function documentation surrounded by """...""" like so

def decrypt(key, ciphertext):
    """Initialize counter for decryption.

    iv should be the same as the output of encrypt().
    """
   #^--- you probably also mean ciphertext here

This will also allow Python's built-in help(...) function to pick up your documentation.

3. Whitespace
You should tidy up the newlines in your code. There are often two (sometimes more) newlines within a function's body, while there are none between the imports and functions. This might be an issue from posting the code here on Code Review, but you should definitely check that. A reasonable convention many people agree on is to use two newlines to seperate functions (or imports from the following functions) and only use a single newline in function bodys to visually create logical groups of code.

The code itself

1. Exceptions
"You're not doing the right thing!" What thing you might ask? Well, ... "Read it! Find it out yourself!"
Sounds frustrating, doesn't it? Python offers a wide variety of built-in exceptions so you can easily convey the type of error that has happened, without the need to parse/read the actual error message. In your case ValueError seems to be appropriate. From the documentation:

  exception ValueError
     Raised when an operation or function receives an argument that has the right type but an inappropriate value, [...]

In case of an error on checking the HMAC, you might add your own exception derived from Exception (maybe HMACValidationError?). I will leave this as an exercise to you.

2. encrypt/decrypt
These functions look quite reasonable from what I can jugde and work the way I would expect them to work. A minor "issue" I found, is that they are not handling string encoding/decoding symmetrically. While encrypt does handle unicode strings by converting them to bytes representation before encryption, decrypt has no way of knowing that this step has taken place and will always return the raw bytes.
At the moment, I would favor an approach where the application using those functions has to take care that the input is already encoded into bytes and also decode the bytes back into a unicode string if desired. This would also be in line with not handling string values for the key, which strongly hints that there is an earlier step where those are created and possibly encoded.

3. Security
From what I've seen on my journey through cryptoland, it is a quite common practice to simply prepend the ciphertext with the initialiation vector (IV). Password hashing functions do something similar and tend to store the salt prepended to the actual hash to have it available for validation later. Remember: the IV does not protect your ciphertext in the same way the key does, and therefore must not be handled with the same care. It instead prevents an attacker to see the same plaintext mapped the same ciphertext repeatedly, which is maybe more than you can afford to reveal.
Something similar applys to the HMAC. The security of the HMAC authentication should only rely on the secrecy of the key, and nothing else. So as long as you use a suitable HMAC implementation and a reasonably secure key, you should be good to go. Just bear in mind that an HMAC alone does not and cannot prevent all kind of attacks, e.g. HMAC alone cannot prevent replay.


Converting a string to coordinates I'm going through the CoffeeScript book by Trevor Burnham, and I was curious as to what the best style is for a function I was writing.

In the book the author writes the function like so:

strToCoordinates = (input) ->
    halves = input.split(',')
    if halves.length is 2
        x = parseFloat halves[0]
        y = parseFloat halves[1]
        if !isInteger(x) or !isInteger(y)
            console.log "Each coordinate must be an integer."
        else if not inRange x - 1, y - 1
            console.log "Each coordinate must be between 1 and #{GRID_SIZE}."
        else
            {x, y}
    else
        console.log 'Input must be of the form `x, y`.'

I wrote my function like so:

strToCoordinates = (input) ->
    halves = input.split(',')
    if halves.length isnt 2
        console.log 'Please enter coordinates in format: x,y'; return
    [x, y] = (parseFloat s for s in halves)
    unless isInteger(x) and isInteger(y)
        console.log 'The coordinates must be integers.'; return
    if not inRange(x, y)
        console.log "Coordinates must be between 1 and #{GRID_SIZE}."; return
    {x, y}

Is my use of just if statements and using return to stop the flow if the check fails an okay style?

I like the fact that with this style the error messages are right after the check unlike with if/else, and also you don't have to have large sections indented.

Well I don't now coffeescript, but guess I can read it anyway :D

I think both are good and readable.
I don't know if you can add empty lines to the code. If you can:

strToCoordinates = (input) ->
    halves = input.split(',')
    if halves.length isnt 2
        console.log 'Please enter coordinates in format: x,y';
        return

    [x, y] = (parseFloat s for s in halves)
    unless isInteger(x) and isInteger(y)
        console.log 'The coordinates must be integers.';
        return

    if not inRange(x, y)
        console.log "Coordinates must be between 1 and #{GRID_SIZE}.";
        return

    {x, y}

But this really is a minor arguable tweak (some will like it, some don't, I do).
Another point would be the "overuse" of returns. Functions are easier to read, if they do have lss return-statements. But Again, this usually applies to larger functions only.


Temperature scale converter in Java I am creating this converter.
There are three units that can be converted: kelvin, celcius and fahrenheit.
I used two variables to check which unit is chosen.
It's cek and cek2.
Is there any way I can shorten this code,
or I just have to add all possible combination using if statements as i am doing now?

 if (cek=="celcius"&&cek2=="celcius"){
  hasil.setText(nilai.getText());
  } //celcius ke celcius

  if (cek=="celcius"&&cek2=="fahrenheit"){
  double hasil1 = (nilai1*9/5)+32;
  String hasil2 =  String.valueOf(n.format(hasil1));
  hasil.setText(hasil2);
  }

  if (cek=="celcius"&&cek2=="kelvin"){
  double hasil1 = nilai1+273.15;
  String hasil2 =  String.valueOf(n.format(hasil1));
  hasil.setText(hasil2);
  }

  if (cek=="fahrenheit"&&cek2=="celcius"){
  double hasil1 = (nilai1-32)*5/9;
  String hasil2 =  String.valueOf(n.format(hasil1));
  hasil.setText(hasil2);
  }
  ....................................

You can shorten that code by not converting the values directly, but rather use common intermediate representation. E.g. you could convert the incoming number first to celsius (not matter what source and destinations are) and then convert the intermediate representation to the target:

double inputTemperature = ... ;
double outputTemperature;
String inputUnit = ... ;
String outputUnit = ... ;
// input conversion if necessary
if (inputUnit.equals("fahrenheit")) {
    inputTemperature = fahrenheitToCelsius(inputTemperature);
}
if (inputUnit.equals("kelvin")) {
    inputTemperature = kelvinToCelsius(inputTemperature);
}
// output conversion:
if (outputUnit.equals("fahrenheit")) {
    inputTemperature = celsiusToFahrenheit(inputTemperature);
}
if (outputUnit.equals("kelvin")) {
    inputTemperature = celsiusToKelven(inputTemperature);
}

So only 4 if statements rather than 9.

P.S.: Please review https://stackoverflow.com/questions/513832/how-do-i-compare-strings-in-java


Generalizing a choose operation Consider the following piece of code.

public static String[] choosePatterns(int total, int toChoose, int numOfPatterns1) {
    String[] patterns = new String[numOfPatterns1];
    int x = 0;
    StringBuilder data = new StringBuilder("");
    List<Integer> indices = IntStream.range(1, toChoose + 1).mapToObj(i -> Integer.valueOf(0)).collect(Collectors.toList());
    resetIndex(indices, 0, 1, total);
    while (true) {

        for (Integer i = 1; i <= total; i++) {
            if (! indices.contains(Integer.valueOf(i))) {
                data.append(i.toString());
            }
        }
        patterns[x++] = data.toString();
        data = new StringBuilder("");

        if (! incrementIndices(indices, indices.size() - 1, total)) {
            break;
        }
    }
    return patterns;
}

public static boolean resetIndex(List<Integer> indices, int posn, int value, int total) {
    if (value <= total) {
        indices.set(posn, value);
        return posn == indices.size() - 1 ? true : resetIndex(indices, posn + 1, value + 1, total);
    } else {
        return false;
    }
}

public static boolean incrementIndices(List<Integer> indices, int posn, int total) {
    if (indices.get(posn) < total) {
        indices.set(posn, indices.get(posn) + 1);
    } else {
        int resetPosn = posn;
        do {
            if (resetPosn-- == 0) return false;
        } while (! resetIndex(indices, resetPosn, indices.get(resetPosn) + 1, total));
    }
    return true;
}

Now, this currently prints out all the remanants of a choose operation on a piece of data. For instance, if I call the method as choosePatterns(4,1, 4) then it will print 234, 134, 124, 123. Similarly, if it was choosePatterns(4,2, 6), then it will print all the possibilities when two data points are removed in the string.

First and foremost, is there a way to generalize this function so that I can input any type of data and output the combination patterns on that? For instance, instead of 1234 being permuted and removed in different ways, what if my original data was ABCD? I am having a hard time being able to alter this method.

Secondly, are there ways to optimize these methods so it runs in the faster time possible?

First and foremost, is there a way to generalize this function so that I can input any type of data and output the combination patterns on that?

Yes, if you supply the alphabet/element set as input to your function, and treat your output as indices into that alphabet.  Right now, your output is a list of numbers per combination (rather, a string, but see later).  This makes them feasible to use as an index into an arbitrary list.

In the least intrusive form, without modifying existing code, you can add this function that will do what you want.  (It's not efficient, though.)

/** Returns a list of all combinations over <var>alphabet</var> of length <var>patternLength</var>. */
public static <T> List<List<T>> choosePatterns(List<T> alphabet, int patternLength) {
    final int n = alphabet.size();
    String[] patterns = choosePatterns(n, n - patternLength, combi(n, patternLength));
    List<List<T>> retval = new ArrayList<>();
    for ( String pattern : patterns ) {
        List<T> list = pattern.chars()
                .map(Character::getNumericValue)
                .map(i -> i - 1)
                .mapToObj(alphabet::get)
                .collect(Collectors.toList());
        retval.add(list);
    }
    return retval;
}

/** n \choose k */
static int combi(int n, int k) {
    int result = 1;
    for (int i = 0; i < k; i++) {
        result *= (n-i);
        result /= (i+1);
    }
    return result;
}

/* Example usage */
public static void main(String[] args) {
    List<String> alphabet = Arrays.asList("C", "A", "T", "G");
    for ( List<String> pattern : choosePatterns(alphabet, 3) ) {
        System.out.println(pattern);
    }

    List<?> numbers = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9);
    for ( List<?> pattern : choosePatterns(numbers, 3) ) {
        System.out.println(pattern);
    }
}

  Secondly, are there ways to optimize these methods so it runs in the faster time possible?

Yes, if only because you're working with Strings/objects where you don't really need to.  But it depends on how far you want to take it.

On the surface, there are a number of inefficiencies that you can eliminate:

Working with List<Integer> rather than int[] : all your lists are fixed-size.  Working with arrays will allow you to use primitives, which will improve performance when making calculations.  (When not making calculations, this difference is still noticeable but much less pronounced.)
Returning String rather than int[] : if you return the indices in directly usuable form, you can avoid having to translate back and forth when using them, and also reduce the number of object allocations.  (Allocations are cheap but not free.)
data = new StringBuilder("") : it's typically cheaper (but not as 'clean') to wipe your current buffer than to allocate a new one.  For reasons beyond my comprehension, StringBuilder does not have a clear() method, but you can use delete(0, xyz.length()).  (Even better to just not use Strings, then this becomes moot.)

Depending on your use case, you can also consider a different approach (*).  Right now, you generate all possible combinations before returning.  If you don't need all combinations at the same time, this may be overkill.  (If you do, then making them all in one go is probably the most efficient.)

(*) Because the set of possible combinations is enumerable, you can assign a unique number to a combination, given an alphabet and a pattern length.  Meaning you can have a mapping function, i → combination(alphabet, k), as the base of an (spl)iterator or even a java.util.List implementation with a very efficient contains operation.  But that may be beyond the scope of this answer.


Square Root Calculator I have now written a simple square root calculator using the division method:

static void Main(string[] args)
{
    double num, sqrt = 0;
    int currentDecimal = 0, decimalAccuracyLevel, intAccuracyLevel = 5;

    do
    {
        Console.Write("Enter your number: ");
    } while (!double.TryParse(Console.ReadLine(), out num));

    do
    {
        Console.Write("Enter the number of decimal points you wish to be accurate to: ");
    } while (!int.TryParse(Console.ReadLine(), out decimalAccuracyLevel) && decimalAccuracyLevel >= 0);

    while (0 < intAccuracyLevel)
    {
        double currentIncrement = Math.Pow(10, intAccuracyLevel);

        while (Math.Pow(sqrt + currentIncrement, 2) <= num)
        {
            sqrt += currentIncrement;
        }

        intAccuracyLevel--;
    }

    while (currentDecimal <= decimalAccuracyLevel)
    {
        double currentIncrement = (double)1 / Math.Pow(10, currentDecimal);

        while (Math.Pow(sqrt + currentIncrement, 2) <= num)
        {
            sqrt += currentIncrement;
        }

        currentDecimal++;
    }

    Console.WriteLine(sqrt);
}

Is there anything I should do to improve performance or accuracy?

Declare your variables a near as possible to their usage.

Initializing multiple variables on a single line removes readability and can lead in some languages to unexpected results.

If you divide two numbers and one of the numbers is a double the result is not a integer division result, hence you don't need the cast to double.

Because you are reinventing the wheel, it would be appropriate to do the Pow() by your code, because otherwise you could simply use Math.Pow(num, 0.5).

You should name your variables better. The int currentDecimal variable is storing the current calculated decimal points, but it reads that it holds the current Decimal number. You should rename it to currentDecimalPoint or something more meaningful.

This is a small application, but you should nevertheless extract the reading of the user input and the calculation of the result to separate methods.


Code for sorting items in recyclerView I'm have recyclerView which is needed for displayed for show list of my audio records. I added the ability to sort my audio records. Please take a look at the code that I use for sorting and tell me how it can be improved.
enum class AudioRecordSort {

    SortByDate {
        override fun sort(empties: List<AudioRecordEmpty>) {
            Collections.sort(empties, AudioRecordDateComparator())
            if (!isIncrease)
                empties.reversed()
        }
    },
    SortByName {
        override fun sort(empties: List<AudioRecordEmpty>) {
            Collections.sort(empties, AudioRecordNameComparator())
            if (!isIncrease)
                empties.reversed()
        }
    },
    SortBySize {
        override fun sort(empties: List<AudioRecordEmpty>) {
            Collections.sort(empties, AudioRecordSizeComparator())
            if (!isIncrease)
                empties.reversed()
        }
    },
    SortByTime {
        override fun sort(empties: List<AudioRecordEmpty>) {
            Collections.sort(empties, AudioRecordTimeComparator())
            if (!isIncrease)
                empties.reversed()
        }
    };

    val isIncrease: Boolean = true

    abstract fun sort(empties: List<AudioRecordEmpty>)
}

I would also like to say that I do not like that in each method I have to call
 if (! IsIncrease)
    empties.reversed ().

Could this be done in a separate place for everyone?

You're right that the duplicate code in the sorters looks bad.
To solve this, each sorter should only define the compare function, and all the rest should be done by the enum class. Or not even this. It took me a little while of experimenting until the code compiled, but here it is:
enum class AudioRecordSort(
        private val cmp: Comparator<AudioRecordEmpty>
) {
    SortByDate(AudioRecordDateComparator()),
    SortByName(AudioRecordNameComparator()),
    SortBySize(AudioRecordSizeComparator()),
    SortByTime(AudioRecordTimeComparator());

    fun sort(empties: MutableList<AudioRecordEmpty>, asc: Boolean) {
        val cmp = if (asc) cmp else cmp.reversed()
        empties.sortWith(cmp)
    }
}

I don't think it can get any simpler than this.
I also fixed the type of the empties parameter. You had List<>, which in Kotlin is an immutable list. I changed that to MutableList<>.
I also fixed the reversed call to be reverse. I bet your IDE has warned you about the "unused result of reversed". This in turn means that you didn't test your code thoroughly before posting it here, since otherwise you would have noticed that it doesn't work for isIncrease == false.
Next time please post complete, compilable code. The missing parts for this question are:
class AudioRecordEmpty

class AudioRecordDateComparator : Comparator<AudioRecordEmpty> {
    override fun compare(o1: AudioRecordEmpty?, o2: AudioRecordEmpty?) = 0
}

class AudioRecordNameComparator : Comparator<AudioRecordEmpty> {
    override fun compare(o1: AudioRecordEmpty?, o2: AudioRecordEmpty?) = 0
}

class AudioRecordSizeComparator : Comparator<AudioRecordEmpty> {
    override fun compare(o1: AudioRecordEmpty?, o2: AudioRecordEmpty?) = 0
}

class AudioRecordTimeComparator : Comparator<AudioRecordEmpty> {
    override fun compare(o1: AudioRecordEmpty?, o2: AudioRecordEmpty?) = 0
}


Code to update content via ajax Following is the code to update content via ajax. Please review all aspects of the code.

var ajaxUpdate = {
    init: function(){
        ajaxUpdate.enableHistory();
    },
    _this:null,
    enableHistory: function(){
        History.Adapter.bind(window,'statechange',function() { // Note: We are using statechange instead of popstate
            var State = History.getState();

            console.log('History: '+State.url)
            if(State.data.type == 'maincontainer'){
                if(jQuery('#maincontainer').length){
                    //update content
                    jQuery('#maincontainer').html(State.data.response);

                    //select the menu element
                    jQuery('#main-menu li').removeClass('active');
                    jQuery('#main-menu li:eq('+State.data.main+')').addClass('active');
                }
                else{
                    window.location.href = State.url;
                }
            }/*
            _this.parent().addClass('active');*/
        });
    },
    updateMainContent: function(){
        var _this = ajaxUpdate._this;
        if ( !History.enabled ) {
            return ;
        }

        jQuery.ajax({
            type: 'GET',
            url: _this.attr('href'),
            beforeSend  : function(jqXHR, settings){
                jQuery('#maincontainer').addClass('loading');
            },
            complete: function(jqXHR, textStatus){
                jQuery('#maincontainer').removeClass('loading');
            },
            success: function(response){
                var data = {
                    main : _this.data('main_index'),
                    type: "maincontainer",
                    response: response
                }
                jQuery('#maincontainer').removeClass('loading');
                History.pushState(data, _this.data('title'), this.url);
            },
            error : function(jqXHR, textStatus, errorThrown){
                //processERROR();
            }
        });
        return false;
    },
    mainmenu: function(){
        ajaxUpdate._this = jQuery(this);
        ajaxUpdate._this.data('main_index', ajaxUpdate._this.parent().index());
        return ajaxUpdate.updateMainContent();
    },
    submenu: function(){
        ajaxUpdate._this = jQuery(this);
        ajaxUpdate._this.data('main_index', jQuery('#main-menu li.active').index());
        return ajaxUpdate.updateMainContent();
    },
    search: function(){
        ajaxUpdate._this = jQuery(this);
        ajaxUpdate._this.data('main_index', jQuery('#main-menu li.active').index());
        return ajaxUpdate.updateMainContent();
    }
};

ajaxUpdate.init();

jQuery('#main-menu a').click(ajaxUpdate.mainmenu);
jQuery('#maincontainer').on('click', '.nav a', ajaxUpdate.submenu);
jQuery('#maincontainer').on('submit', 'form', ajaxUpdate.search);

A couple things right off the bat:

You use jQuery('#maincontainer') multiple times. You should consider caching that somewhere. Either as part of your object or a separate variable.

// Wherever you use jQuery('#maincontainer'), use ajaxUpdate.mainContainer
ajaxUpdate.mainContainer = jQuery('#maincontainer');

I would suggest passing the this from your event handlers directly to updateMainContent since that is the only method that uses it.
Unless you have plans to significantly change the way events are handled between search, submenu, and mainmenu, you can can just have one method that handles those events.
complete gets called whether it's an error or success. No need to remove the loading class on success and complete.
Your beforeSend function is unnecessary since it doesn't actually manipulate any of the data before the AJAX call or cancel the AJAX call. You can move the addClass to before the jQuery.ajax. From the jQuery docs:

  A pre-request callback function that can be used to modify the jqXHR (in jQuery 1.4.x,
  XMLHTTPRequest) object before it is sent. Use this to set custom headers, etc. ...
  Returning false in the beforeSend function will cancel the request.

You can chain the following lines together (to remove another call to jQuery):

jQuery('#main-menu li').removeClass('active');
jQuery('#main-menu li:eq('+State.data.main+')').addClass('active');

Like so:

jQuery('#main-menu li').removeClass('active')
    .filter(':eq('+State.data.main+')').addClass('active');

Switching your AJAX call to a .get and using the promise API cleans up the code a little and makes it a bit more concise.
In your success response, you don't need to define data since you're just passing it directly to History.pushstate.

Not knowing the HTML structure, I think that's the best I can give you. Here's a fiddle with all of these things implemented.

I'm more than happy to answer any questions if you have them. Hopefully this will help you out.


Getting maximum values in array There is a function which is getting maximum values of each period-length interval in array.
I am trying to optimize this function by performance.

void maximumsT(const std::vector<double> &src, size_t period, std::vector<double> &dst)
{
    dst.resize(src.size());

    for (size_t i = 1; i <= period; ++i) {
        dst[i - 1] = *std::max_element(src.begin(), src.begin() + i);
    }

    for (size_t i = period; i <= src.size(); ++i) {
        dst[i - 1] = *std::max_element(src.begin() + i - period, src.begin() + i);
    }
}

I asked this SO question
and got this solution.

Then I tried to implement the solution.

#include <vector>
#include <deque>
#include <algorithm>

void maximumsT(const std::vector<double> &src, size_t period, std::vector<double> &dst)
{
    dst.resize(src.size());

    for (size_t i = 1; i <= period; ++i) {
        dst[i - 1] = *std::max_element(src.begin(), src.begin() + i);
    }

    for (size_t i = period; i <= src.size(); ++i) {
        dst[i - 1] = *std::max_element(src.begin() + i - period, src.begin() + i);
    }
}

void maximums(const std::vector<double> &src, size_t period, std::vector<double> &dst)
{
    dst.resize(src.size());
    std::deque<std::pair<size_t, double>> deq;

    for (size_t i = 0; i < src.size(); ++i) {
        if (!deq.empty() && i >= period && deq.front().first <= i - period) {
            deq.pop_front();
        }

        while (!deq.empty() && deq.back().second < src[i]) {
            deq.pop_back();
        }

        deq.push_back(std::make_pair(i, src[i]));
        dst[i] = deq.front().second;
    }
}

bool test()
{
    std::vector<double> v(1 + rand() % 1000);
    for (double &e : v) {
        e = (double)rand() / RAND_MAX;
    }

    size_t period = 1 + (rand() % v.size());
    std::vector<double> vv, vvv;

    maximumsT(v, period, vv);
    maximums (v, period, vvv);

    return vv == vvv;
}

bool test(size_t iterations)
{
    for (size_t i = 0; i < iterations; ++i) {
        if (!test()) {
            return false;
        }
    }

    return true;
}

int main(int argc, char *argv[])
{
    return test(10000);
}

How can I improve and optimize this code?

The code in maximums is doing what it is supposed to do, based on that SO question. I have several remarks about the other parts though.

Do not use single letter variables. Use names that say something.
So no v, vv, vvv.

Another naming issue, your two maximums functions do the same thing but with a different approach, you can make this clear in the name. They both use T=period, so this is not distinguishing. Maybe use the fact that a deque is used or that one is the simple variant.

You create double vectors in the test and then pass them to maximums for example. There is absolutely no reason for test to create them. Let maximums create the vector, since it needs it and then return it. Some for maximumsT.

Your maximumsT is not intuitive. There is no need to do a double loop.
Why did you change your implementation as in your question on SO? It was better there.
Namely

for (size_t i = period; i <= v.size(); ++i) {
        vv[i - 1] = *std::max_element(v.begin() + i - period, v.begin() + i);
    }

The code will not be faster or save space, but its shorter and clearer.

Initializing random vectors to test against, could be done in a dedicated function. I would make the test also parameterized on the source vector.

My suggestion:

#include <vector>
#include <deque>
#include <algorithm>

std::vector<double> maximumsT(const std::vector<double> &src, size_t period)
{
    std::vector<double> dst(src.size());

    for (size_t i = period; i <= v.size(); ++i) {
       vv[i - 1] = *std::max_element(v.begin() + i - period, v.begin() + i);
    }
    return dst;
}

std::vector<double> maximums(const std::vector<double> &src, size_t period)
{
    std::vector<double> dst(src.size());
    std::deque<std::pair<size_t, double>> deq;

    for (size_t i = 0; i < src.size(); ++i) {
        if (!deq.empty() && i >= period && deq.front().first <= i - period) {
            deq.pop_front();
        }

        while (!deq.empty() && deq.back().second < src[i]) {
            deq.pop_back();
        }

        deq.push_back(std::make_pair(i, src[i]));
        dst[i] = deq.front().second;
    }
    return dst;
}

std::vector<double> randomVect()
{
    std::vector<double> v(1 + rand() % 1000);
    for (double &e : v) {
        e = (double)rand() / RAND_MAX;
    }
    return v
}
bool compareMaximums(std::vector<double> v, size_t period)
{
    return maximumsT(v, period) == maximums(v, period);
}

bool test(size_t iterations)
{
    for (size_t i = 0; i < iterations; ++i) {
        std::vector<double> srcVec = randomVect();
        size_t period = 1 + (rand() % srcVec.size());
        if (!compareMaximums(srcVec, period)) {
            return false;
        }
    }

    return true;
}

int main(int argc, char *argv[])
{
    return test(10000);
}


Simple React component to show sorted articles I have a basic articles sorting application, the App component gets some articles and pass them to the Articles component which renders a list of the articles sorted by date or by upvotes.
Each article is an object with id (string), title (string), upvotes (number), and date (string format YYYY-MM-DD) properties.
By default the articles are shown by upvotes in descending order.
This is the Articles component:
import React from "react";

function Articles({ articles }) {
  return (

            Title
            Upvotes
            Date

          {articles.map(({id, title, upvotes, date}) => (

              {title}
              {upvotes}
              {date}

          ))}

  );
}

export default Articles;

And this is the App component:
import React, { useState } from 'react';
import './App.css';

import Articles from './components/Articles';

function App({ articles }) {

  const sortByUpvotesDesc = (a,b) => b.upvotes - a.upvotes
  const sortByDateDesc = (a,b) => b.date > a.date ? 1 : b.date === a.date ? 0 : -1

  const [articlesOrder, setArticlesOrder] = useState('byUpvotesDesc')

  const mostUpvotedClickHandler = () => {
    setArticlesOrder('byUpvotesDesc')
  }

  const mostRecentClickHandler = () => {
    setArticlesOrder('byDatesDesc')
  }

  return (

        Sort By
        <button onClick={mostUpvotedClickHandler} >Most Upvoted</button>
        <button onClick={mostRecentClickHandler} >Most Recent</button>

      <Articles articles={articles.sort(articlesOrder === 'byUpvotesDesc' ? sortByUpvotesDesc : sortByDateDesc)} />

  );

}

export default App;

I would appreciate any insights and feedback on how to improve this, especially regarding the App component.

Date sorting Your dates are of the format YYYY-MM-DD. You currently sort them with:
(a, b) => b.date > a.date ? 1 : b.date === a.date ? 0 : -1

When you want to lexiographically compare strings to determine which comes first in the alphabet for sorting, you can do it more concisely with localeCompare, which returns a number indicating whether one string comes before, is the same as, or after another. You can use:
(a, b) => b.date.localeCompare(a.date);

articlesOrder's state relies on, essentially, magic strings:
const [articlesOrder, setArticlesOrder] = useState('byUpvotesDesc')

Since there are only two possible sorting methods, you might consider using a boolean instead:
const [orderByUpvotes, setOrderByUpvotes] = useState(true)

If there were more sorting methods, I would feel a bit more comfortable using an enum of sorts (or TypeScript to enforce string comparison safety) than comparing strings like articlesOrder === 'byUpvotesDesc' that need to match across multiple places in the code. For example:
const SORTING_OPTIONS = {
  byUpvotesDesc: 0,
  byDatesDesc: 1,
  // ...
};
// ...
const [articlesOrder, setArticlesOrder] = useState(SORTING_OPTIONS.byUpvotesDesc);

Another option would be to omit the articlesOrder variable entirely, and instead only sort the articles when a button is clicked (see below) - but if you want to expand on the component, keeping an articlesOrder variable will probably be useful to indicate to the user what the current sorting order is (for example, to give the top container a visual indicator of what option is currently selected).
Mutation You are mutating the prop when you do:
<Articles articles={articles.sort(articlesOrder === 'byUpvotesDesc' ? sortByUpvotesDesc : sortByDateDesc)} />

because .sort sorts in-place. Put the prop into state instead, and when a new sort order needs to occur, copy the array before sorting it so as not to mutate. Also, rather than .sorting every time the component is rendered, it would be better to sort only when the sort order changes. Here's one possible approach:
const sortByUpvotesDesc = (a, b) => b.upvotes - a.upvotes
const sortByDateDesc = (a, b) => b.date.localeCompare(a.date);

function App({ articles: articlesProp }) {
    const [articles, setArticles] = useState(articlesProp);
    const [articlesOrder, setArticlesOrder] = useState();

    const sortArticles = (callback) => {
        setArticles(
            articles => articles.slice().sort(callback)
        );
    };
    // The below could be made more abstract and DRY if there were more options
    // but with only two, it would probably be more confusing than useful
    const mostUpvotedClickHandler = () => {
        sortArticles(sortByUpvotesDesc);
        setArticlesOrder(SORTING_OPTIONS.byUpvotesDesc);
    };
    const mostRecentClickHandler = () => {
        sortArticles(sortByDateDesc);
        setArticlesOrder(SORTING_OPTIONS.byDatesDesc);
    };
    // Sort articles on initial mount, before they're rendered, to avoid reflow:
    useLayoutEffect(mostUpvotedClickHandler, []);

    return (

                Sort By
                <button onClick={mostUpvotedClickHandler} >Most Upvoted</button>
                <button onClick={mostRecentClickHandler} >Most Recent</button>

            <Articles articles={articles} />

    );
}

Semicolons Best to be consistent; either use semicolons or don't. If you're skilled and confident that you won't be bitten by the rules of ASI, you can omit them. Use a linter to enforce your desired style across your codebase.


Calculating the prime numbers between two integers I've wrote this code to calculate the prime numbers between two integers n and m, and I want to optimise it.

#include<iostream>
#include <cmath>
using namespace std;
int main (){
long int T[100][2],n,m,i,j;
int testcase,k;
bool prem;
cin>>testcase;
for(i=0;i<testcase;i++){
   cin>>T[i][0]>>T[i][1];}
for(k=0;k<testcase;k++){
    m=T[k][0];
    n=T[k][1];
for(i=m;i<=n;i++){
    prem=true;
for(j=2;j<i;j++){
    if(i%j==0){prem=false;}}
    if(prem) {cout<<i<<endl;}}}
return 0;}

Use better variable names. Or, alternatively, include comments to show what the variables are for.
Reformat the code with better indentation.
Do not use using namespace std. That is just bad practice.
Instead of looping j from 2 to i, you can just loop upto floor(sqrt(i) + 1).
Do not define i, j and k for such a large scope. Keep their scopes minimal.
Since your inputs would always be positive integers, use unsigned.
This is just a personal convention, and you're not bound to it. I like to define variables in the order bool, char, int, long int, float, double, ... so on.

# include <iostream>
# include <cmath>

# define ULI unsigned long int

int main () {
    bool is_prime;
    int num_cases;
    ULI cases[100][2];
    std::cin>>num_cases;

    for(int i = 0; i < num_cases; i++)
       std::cin >> cases[i][0] >> cases[i][1];
    for(int k = 0; k < num_cases; k++) {
        ULI m = cases[k][0],
            n = cases[k][1];
        for(ULI i = m; i <= n; i++) {
            is_prime = true;
            ULI max = (ULI) sqrt(i) + 1;
            for(ULI j = 2; j < i; j++)
                if(i % j == 0)
                    is_prime = false;
            if(is_prime)
                std::cout<<i<<std::endl;
        }
    }
    return 0;
}


Decimal to binary conversion in ARM assembly I'm "playing" with ARM assembly on a Raspberry Pi, reading various tutorials and posts to help - no, it's not a college assignment, I'm too old for college! I'm pleased with getting the following code to take a decimal value (between 0 and 255) and output the binary string equivalent to the screen using scanf() and printf() but curious to know though whether those more experienced would consider this a "naive" solution? Are there any "gotchas"? Any better ways of tackling the problem?
I reckon there should be a better way of declaring the string to hold the output and how the string is built from the LSB (using R9)
/* dectobin2.s                                                  */
/* ------------------------------------------------------------ */
/* Converts a decimal number to binary and outputs to screen    */
/* ============================================================ */

.data
.balign 4
value:      .word   0
binStr:     .asciz  "00000000"
output:     .asciz  "%d in binary is %s\n"
title:      .asciz  "DECIMAL TO BINARY CONVERTER\n\n"
lr_temp:    .word   0
lr_local:   .word   0
prompt:     .asciz  "Enter a positive value (0-255) > "
pattern:    .asciz  "%d"

.text
@ function to convert given value into a binary string
decToBin:
    @ save the link register
    ldr     r2, =lr_local
    str     lr, [r2]
    @ set data values
    mov     r4, r0              @ value in r4 for processing
    mov     r9, #7              @ LSB position in output string
_loop:
    @ repeated division by 2 2
    movs    r4, r4, lsr #1      @ divide by 2, set carry
    bcs     _odd                @ if carry set, remainder is 1
_even:
    mov     r5, #0              @ remainder of 0
    b       _toString
_odd:
    mov     r5, #1              @ remainder of 1
_toString:
    add     r5, #48             @ convert remainder to ASCII
    ldr     r1, =binStr         @ address of binary string
    strb    r5, [r1, r9]        @ store remainder in string at pos in r9
    sub     r9, r9, #1          @ move to next position in output string
    cmp     r4, #0              @ reached zero?
    ble     _endloop
    b       _loop

_endloop:
    @ restore the link register
    ldr     lr, =lr_local
    ldr     lr, [lr]
    bx      lr

.global main

main:
    @ store the link register
    ldr     r1, =lr_temp
    str     lr, [r1]

    @ print a title
    ldr     r0, =title
    bl      printf
_getValue:
    @ get the value to convert
    ldr     r0, =prompt         @ prep the arguments for scanf
    bl      printf
    ldr     r0, =pattern
    ldr     r1, =value
    bl      scanf

    @ check for valid input (0-255)
    ldr     r0, =value
    ldr     r0, [r0]
    cmp     r0, #255            @ must be <= 255
    bgt     _getValue
    cmp     r0, #0              @ must be >= 0
    blt     _getValue

_isValid:
    @ call the function, value is in R0
    bl      decToBin

    @ display output
    ldr     r0, =output         @ get address of output string
    ldr     r1, =value          @ get the original value
    ldr     r1, [r1]            @ for first parameter
    ldr     r2, =binStr         @ get the binary string
    bl      printf              @ print the result

    @ restore the link register and exit
    ldr     lr, =lr_temp
    ldr     lr, [lr]
    bx      lr

.global printf
.global scanf
```

Here are a number of things that may help you improve your program.
Understand the calling convention
The procedure call standard for ARM says that the registers r0, r1, r2, and r3 can be used and not restored by a called function.  Further, if we don't alter lr, there's no need to preserve and then restore it.  These are all clues for making your program more efficient.
Know your instruction set
The code currently contains these lines:
    movs    r4, r4, lsr #1      @ divide by 2, set carry
    bcs     _odd                @ if carry set, remainder is 1
_even:
    mov     r5, #0              @ remainder of 0
    b       _toString
_odd:
    mov     r5, #1              @ remainder of 1
_toString:
    add     r5, #48             @ convert

That is terribly inefficient.  We can simplify considerably:
    mov     r5, #48             @ start with ASCII '0'
    movs    r4, r4, lsr #1      @ divide by 2, set carry
    adc     r5, #0              @ add zero + carry

Minimize branching
The code ends with this curious combination:
    ble     _endloop
    b       _loop
_endloop:

First, the comment suggests that we're looking for exactly zero and so we should be looking only at the zero flag (beq or bne) rather than doing a signed comparison with ble.  Second, why not simply use a single instruction?
    beq     _loop

However, better, see the next suggestion.
Create reusable code
The code currently exits if the shifted value becomes zero.  That works, but only once.  In other words, if you use the routine to decode 255 and then again to decode the value 3, you will likely be surprised that your routine will falsely give the same binary string for both.  The problem is that it relies on the binStr buffer to always contain a string of zeroes.  A better approach is to explicitly set each byte of the buffer and instead use the counter you already have in r9.  This shortens the code and makes it reusable.
    subs    r9, r9, #1
    bpl     _loop

Move loop invariants outside the loop
Every iteration through the loop we load the r1 register with the address of binStr even though r1 is never changed within the loop.  A more efficient approach would be to load r1 once outside the loop.
Document register usage
One of the keys to being a good assembly language programmer is to carefully manage register use.  The most basic requirement is to keep track of how you're using registers, and a good way to do that, both for yourself and future readers of the code is to document the use.  Based on the observations in the first suggestion above, here's what I used:
@ Register usage:
@   r0 - passed value
@   r1 - pointer to current string digit
@   r2 - decrementing digit offset
@   r3 - the current digit

Results
Using all of these suggestions, the resulting routine is only nine instructions.
decToBin:
    ldr     r1, =binStr         @ output string location
    mov     r2, #7              @ start with the LSB
_loop:
    mov     r3, #48             @ start with ASCII zero
    movs    r0, r0, lsr #1      @ divide by 2, set carry
    adc     r3, #0              @ add the extracted bit
    strb    r3, [r1, r2]        @ store remainder in string at pos in r9
    subs    r2, r2, #1          @ decrement counter
    bpl     _loop               @ end if it has gone negative
    bx      lr

Armed with these suggestions (pun intended!) see if you can perform a similar cleanup on the rest of the code.  In particular, there is a much more efficient way to check if the input value is in the appropriate range (hint: movs r1, r0, lsr #8).
Also, just for fun, this answer was composed and tested entirely on a Raspberry Pi running on battery power.
Other ideas
A more generally useful routine would have the caller pass a buffer pointer and buffer length into which the program would put its result rather than using a fixed location.  See if you can figure out how to modify the code to do that, and don't forget to place the terminating \0 character.


Could this ExecuteScalar call be written better? I came across this code in our project today.  Where possible I'm trying to leave the code base in a better shape than I found it, as I go along, and this method jumped out at me for a number of reasons, mainly the sql string and the try/catch block.  I feel there's a less expensive way to do it.

Original Code:

public bool CheckSomething(string paramA, int paramB)
{
    using (var conn = new SqlConnection(Connection))
    {
        conn.Open();
        string sqlCommand = "SELECT ColumnA FROM OurTable WHERE ColumnB = '" + paramA +
                                "' AND ColumnC = " + paramB;

        using (var dbCommand = new SqlCommand(sqlCommand, conn))
        {
            int noOfRecords = -1;
            try
            {
                noOfRecords = (int)dbCommand.ExecuteScalar();
            }
            catch (Exception ex)
            {
            }
            finally
            {
                dbCommand.Dispose();
                if (conn.State == ConnectionState.Open)
                {
                    conn.Close();
                }

                return noOfRecords > 0;
            }
        }
    }
}

I was thinking of re-writing it as this, but I still think it could be improved further, one of which would be to create an procedure for the sql, but that's unlikely.  Was aiming to improve it purely from the code point of view.  I'd appreciate thoughts.

Rewritten version:

public bool CheckSomething(string paramA, int paramB)
{
    using (var conn = new SqlConnection(Connection))
    {
        conn.Open();
        string sqlCommand = string.Format("SELECT ColumnA FROM OurTable WHERE ColumnB = '{0}' and ColumnB = {1}", paramA, paramB);

        using (var dbCommand = new SqlCommand(sqlCommand, conn))
        {
            object noOfRecords = dbCommand.ExecuteScalar();
            dbCommand.Dispose();

            if (conn.State == ConnectionState.Open)
            {
                conn.Close();
            }

            return noOfRecords != null;
        }
    }
}

public bool CheckSomething(string paramA, int paramB)
{
    using (var conn = new SqlConnection(".."))
    using (var comm = new SqlCommand("", conn))
    {
        conn.Open();
        object noOfRecords = comm.ExecuteScalar();
        return noOfRecords != null;
    }
}

There is no need to close or dispose, the using handles that part.  This removes the need for a manual try catch or closing logic, leaving a much compressed chunk of code that is functionally equivalent and just as safe.

As for the select statement itself, either use parameterized SQL or a stored procedure as opposed to string concatenation.  Parameterized SQL:

string sql = "SELECT ColumnA FROM OurTable WHERE ColumnB = @param1 AND ColumnC = @param2";

using (var comm = new SqlCommand(sql, conn))
{
    comm.Parameters.AddWithValue("@param1", param1);
    comm.Parameters.AddWithValue("@param2", param2);

    conn.Open();
    // etc...
}


Async Log4net logging handler - High CPU usage on async call I've got a LoggingMessageHandler that incoming requests on my WebApi application filter through.

public class LoggingMessageHandler : DelegatingHandler
{
    public ILogManager LogManager { get; set; } //injected

    protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, System.Threading.CancellationToken cancellationToken)
    {
        var logger = SetupLogger(request);

        var correlationId = request.GetCorrelationId().ToString();

        var requestInfo = await LogRequest(request, logger, correlationId);

        var response = await base.SendAsync(request, cancellationToken);

        await LogResponse(response, logger, correlationId, requestInfo);

        response.Headers.Add("X-Correlation-Id", correlationId);

        return response;
    }

    private ILogger SetupLogger(HttpRequestMessage request)
    {
        var loggerName = request.RequestUri.AbsolutePath.Replace("/", "-");
        var logger = LogManager.GetLogger(loggerName);
        return logger;
    }

    private async Task<string> LogRequest(HttpRequestMessage request, ILogger logger, string correlationId)
    {
        var requestInfo = string.Format("{0} {1}", request.Method, request.RequestUri);

        var requestMessage = "";

        if (request.Content != null)
            requestMessage = await request.Content.ReadAsStringAsync();

        await LogMessageAsync(logger, "Request", correlationId, requestInfo, requestMessage);
        return requestInfo;
    }

    private async Task LogResponse(HttpResponseMessage response, ILogger logger, string correlationId, string requestInfo)
    {
        var responseMessage = string.Empty;

        if (response.IsSuccessStatusCode)
        {
            if (response.Content != null)
                responseMessage = await response.Content.ReadAsStringAsync();
        }
        else
            responseMessage = response.ReasonPhrase;

        await LogMessageAsync(logger, "Response", correlationId, requestInfo, responseMessage);
    }

    private async Task LogMessageAsync(ILogger logger, string type, string correlationId, string requestInfo, string body)
    {
        await Task.Run(() => logger.Info("{0} - {1} - API - {2}: \r\n{3}", correlationId, requestInfo, type, body));
    }
}

When I run ANTS profiler, and test my application, > 70% of the CPU time is spent on this one line:

await Task.Run(() => logger.Info("{0} - {1} - API - {2}: \r\n{3}", correlationId, requestInfo, type, body));

Is that expected?

Am I doing something wrong?

I haven't used Autofac, but with Ninject you can use the logging extensions to automagically inject an ILogger into any class' constructor, so you can stick to constructor injection (which is always preferable over all other kinds of dependency injection techniques), with the only drawback that you have a using Ninject.Extensions.Logging; dependency on your module (given that your current code already has dependencies on Log4Net, I don't see it being an issue though).

I haven't used Log4Net either, but you might be reinventing the wheel.

With NLog it looks like you simply put an async attribute in your targets configuration and you're set:

<targets async="true">

And with the IoC container auto-resolving ILogger for you, all you really need is to constructor-inject an ILogger into whatever class requires logging functionality.

I would look into implementing similar functionality with Autofac/Log4Net, or see what I get with Ninject/Log4Net or Ninject/NLog logging extensions, both available through NuGet.


Replacing from a list chars in a string Often I need to clean a file name using Path.GetInvalidFileNameChars(), however I do not know of a way to search for any of the invalid letters (except by using Regex) in one pass.

public string LoopMethod()
{
    StringBuilder sb = new StringBuilder(fileName);
    foreach(var invalidChar in Path.GetInvalidFileNameChars())
    {
        sb.Replace(invalidChar, '_');
    }
    return sb.ToString();
}

Regex invalidCharsRegex;
public void RegexMethodInit()
{
    var invalidChars = Path.GetInvalidPathChars().Select(c => Regex.Escape(c.ToString())).ToString();
    invalidCharsRegex = new Regex(string.Join("|", invalidChars));
}

public string RegexMethod(string fileName)
{
    return invalidCharsRegex.Replace(fileName, "_");
}

Is one of those ways the "correct" way to to this or is there a better function I am missing?

foreach(var c in Path.GetInvalidPathChars())
    path = path.replace(c, '_')

That's a bit inefficient as it can allocate a string on every itteration, but usually not a problem. Alternative:

var chars = Path.GetInvalidPathChars()
path = new string(path.Select(c => chars.Contains(c) ? '_' : c).ToArray())


"MathChallenge" game for sums with 4 operators I am trying to start learning Python using Learn Python the Hard Way. I wrote my first game that works as far as tested. It was done as an exercise for this.

Please give me some constructive criticism on this program on sections and areas where I can improve. I realise that my object-oriented programming specifically, and my programming in general, still needs lots of work:

"""
MathChallenge:
A game randomly creating and testing math sums with 4 operators: addition, subtraction, multiplication and division.
Difficulty levels are incremented after each correct answer.
No more than 3 incorrect answers are accepted.
"""

from random import randint, choice

class Math(object):
    """Class to generate different math sums based on operator and difficulty levels"""

    def __init__(self):
        """To initialise difficulty on each run"""
        self.difficulty = 1

    def addition(self, a, b):
        """To return 'addition', '+' sign , and answer of operation"""
        return ('addition', '+', a+b)

    def subtraction(self, a, b):
        """To return 'subtraction', '-' sign , and answer of operation"""
        return ('subtraction', '-', a-b)

    def multiplication(self, a, b):
        """To return 'multiplication', '*' sign , and answer of operation"""
        return ('multiplication', '*', a*b)

    def division(self, a, b):
        """To return 'division', '/' sign , and answer of operation"""
        return ('division', '/', a/b)

    def mathsum(self, difficulty):
        """Function that generates random operator and math sum checks against your answer"""

        print "Difficulty level %d." % difficulty

        #let's initialize some random digits for the sum
        a = randint(1,5)*difficulty
        b = randint(1,5)*difficulty

        #Now let's choose a random operator
        op = choice(self.operator)(a, b)

        print "Now lets do a %s calculation and see how clever you are." % op[0]
        print "So what is %d %s %d?" % (a, op[1], b)

        correct = False
        incorrect_count = 0                         #No more than 3 incorrect answers
        while not correct and incorrect_count<3:
            ans = int(raw_input(">"))

            if ans == op[2]:
                correct = True
                print "Correct!"
                self.difficulty += 1
                self.mathsum(self.difficulty)       #Restart the function with higher difficulty

            else:
                incorrect_count += 1
                if incorrect_count == 3: print "That's 3 incorrect answers.  The end."
                else: print "That's not right.  Try again."

class Engine(Math):
    """Game engine"""

    def __init__(self):
        """To initialise certain variables and function-lists"""

        #Initialise list of math functions inherited to randomly call from list
        self.operator = [
            self.addition,
            self.subtraction,
            self.multiplication,
            self.division
            ]

        #Initialise difficulty level
        self.difficulty = 1

        print "Welcome to the MathChallenge game. I hope you enjoy!"

    def play(self):
        """To start game"""

        #start game
        self.mathsum(self.difficulty)

        #print returned difficulty level achieved
        print "Difficulty level achieved: ", self.difficulty

# Start game
game = Engine()
game.play()

Some notes:

You took a OOP approach. I won't say that's wrong (maybe it was even encouraged by that guide), but when learning a language like Python, where OOP is not compulsory, I'd say it preferable to take a non-OOP approach first.
return ('addition', '+', a+b). Those 4 operations are pretty uniform, why don't you use a data structure instead? (a list, a dictionary). Use module operator.
You code is for version 2.x. Unless you have some good reason, it's better to use Python 3.x, you won't lose a second re-learning things later on.
op[0], op[1], op[2]. If I had to highlight one single aspect of good programming practices, this would be: be declarative! If your code does not look as if you are writing pseudocode (specially if you are using a high-level language), then you're doing something wrong. What's op[1]? No idea, we'd have to look at the function. So destructure the result giving meaningful names to the variables: op_name, op_symbol, op_function = choice(self.operator)(a, b).
I'd take a more functional approach, try to organize code so there are as few in-place updates of variables as possible (not always feasible or idiomatic in an imperative language like Python).

I'd write:

import operator
import random

operations = {
    "addition": ("+", operator.add),
    "substraction": ("-", operator.sub),
    "multiplication": ("*", operator.mul),
    "division": ("/", operator.floordiv),
}

def ask_operation(difficulty, maxtries=3):
    maxvalue = 5 * difficulty
    x = random.randint(1, maxvalue)
    y = random.randint(1, maxvalue)
    op_name, (op_symbol, op_fun) = random.choice(list(operations.items()))
    result = op_fun(x, y)

    print("Difficulty level %d" % difficulty)
    print("Now lets do a %s calculation and see how clever you are." % op_name)
    print("So what is %d %s %d?" % (x, op_symbol, y))

    for ntry in range(1, 1+maxtries):
        answer = int(input(">"))
        if answer == result:
            print("Correct!")
            return True
        elif ntry == maxtries:
            print("That's %s incorrect answers.  The end." % maxtries)
        else:
            print("That's not right.  Try again.")
    return False

def play(difficulty):
    while ask_operation(difficulty):
        difficulty += 1
    print("Difficulty level achieved: %d" % difficulty)

play(1)