Checking if two strings are anagrams in Python The code written in Python 3.6 mostly using Python's built in functions sorted and len. First I'm checking for the edge case that the two given strings are not of the same length obviously. Then I'm sorting the strings and comparing if their sorted value are equal to each other with a boolean expression. An anagram is a word or phrase formed by rearranging the letters of a different word or phrase, typically using all the original letters exactly once.

Just for the sake of practising Big-O this function runs in \$O(1)\$ constant complexity time because regardless of the strings given the function will always return a Boolean value. Is this correct? I realized my assumption was not correct since the function will run in the time complexity of the sorted built-in function which is \$nlog(n)\$.
If you were an interviewer would you prefer the candidate to not use Python's built in functions and to resort to a more manual way of solving the problem?

def is_anagram(string1, string2):

    while len(string1) == len(string2):

    # Testing sorted values equality
        if sorted(string1) == sorted(string2):
            return True
        return False

    return 'The strings are not anagrams they have differing lengths'

print(is_anagram('cat', 'cra'))

You can change your if to just be return.
You should change your while to an if, as it makes no sense for it to be while.
You shouldn't return a string on invalid input, instead you could raise a ValueError.

This can get:

def is_anagram(string1, string2):
    if len(string1) == len(string2):
        return sorted(string1) == sorted(string2)
    raise ValueError('The strings are not anagrams they have differing lengths')

However I wouldn't raise, and so you can just use:

def is_anagram(string1, string2):
    return sorted(string1) == sorted(string2)

To answer your questions:

The \$O\$ complexity for your function is not \$O(1)\$. Lets take the different aspects of your function:

len(n) is \$O(1)\$
int == int is \$O(1)\$
sorted(n) is \$n\log{n}\$
str == str is \$O(n)\$
len(a) == len(b) is \$O(1)\$
sorted(a) == sorted(b) is \$O(\min(a, b) + a\log{a} + b\log{b})\$

Since the function will short circuit if len(a) == len(b) we know that \$a = b = n\$.
And so the complexity becomes \$O(n + n\log{n})\$. This may be ok to simplify to \$O(n\log{n})\$, as \$O(n + n\log{n}) = O(n(1 + \log{n}))\$

You can however use collections.Counter to reduce your sorted complexity to \$O(n)\$. And so keeping the short circuiting your function would be \$O(n)\$, otherwise it would be \$O(a + b)\$. And so to use this you can use:

from collections import Counter

def is_anagram(string1, string2):
    return Counter(string1) == Counter(string2)

I would prefer you use the builtin functions, as it should lead to less code to maintain.


C++ cleaner optimised code, Calculator This code currently works i wonder if i could make any optimisations or anything to make the code appear cleaner using functions or whatnot, suggestions are welcome!

#include <cmath>
#include <cstdio>

int prompt(const char* name)
{
printf("%s", name);
int value;
scanf("%d", &value);
return value;
}

int main()
{
    int mod, a, b, zbroj, razlika, umnozak, kolicnik;
    printf("Mod 1 je za zbrajanje \n");
    printf("Mod 2 je za oduzimanje \n");
    printf("Mod 3 je za mnozenje \n");
    printf("Mod 4 je za dijeljenje \n");
    printf("U kojem modu zelis biti: \n");
    std::scanf("%d", &mod);

    switch (mod)
    {
        case 1:
            printf("Odabrali ste zbrajanje! \n");
            a = prompt("a: ");
            b = prompt("b: ");
            zbroj = a + b;
            printf("%d + %d = %d", a, b, zbroj);
            break;
        case 2:
            printf("Odabrali ste oduzimanje! \n");
            a = prompt("a: ");
            b = prompt("b: ");
            razlika = a - b;
            printf("%d - %d = %d", a, b, razlika);
            break;
        case 3:
            printf("Odabrali ste mnozennje! \n");
            a = prompt("a: ");
            b = prompt("b: ");
            if(a == 0 or b==0){
                printf("Mnozite s nulom, a kad se mnozi s nulom rezultat je uvijek 0: \n");
                umnozak = 0;
            }
            else
            {
                umnozak = a * b;
            }
            printf("%d * %d = %d", a, b, umnozak);
            break;
        case 4:
            printf("Odabrali ste dijeljenje! \n");
            a = prompt("a: ");
            b = prompt("b: ");
            if(a == 0 or b==0){
                printf("Dijelite s nulom, a kad se dijeli s nulom rezultat je uvijek 0: \n");
                kolicnik = 0;
            }
            else
            {
                kolicnik = a / b;
            }
            printf("%d / %d = %d", a, b, kolicnik);
            break;
        default:
            printf("Nemamo tu opciju trenutno, prcekajte za nadogradnju programa, ili se javite developeru! \n");
    }
    return 0;
}

Here are some things that may help you improve your code.

Check return values for errors

The call scanf can fail.  You must check the return values to make sure it hasn't or your program may crash (or worse) when given malformed input or due to low system resources.  Rigorous error handling is the difference between mostly working versus bug-free software.  You should strive for the latter.

Decompose your program into functions

Almost all of the logic here is in main in one rather long and dense chunk of code.  It would be better to decompose this into separate functions.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.  Know when to use it and when not to (as when writing include headers).  In this particular case, I happen to think it's perfectly appropriate because it's a single short program that and not a header. Some people seem to think it should never be used under any circumstance, but my view is that it can be used as long as it is done responsibly and with full knowledge of the consequences.

Use <cstdio> instead of <stdio.h>

The difference between the two forms is that the former defines things within the std:: namespace versus into the global namespace.  Language lawyers have lots of fun with this, but for daily use I'd recommend using <cstdio>.  See this SO question for details.

Prefer iostream to old-style printf

The iostream library is the more modern C++ way to do I/O.  Prefer it over printf and scanf.  So instead of this:

printf("%d + %d = %d", a, b, zbroj);

Write this:

std::cout << a << " + " << b << " = " << zbroj << '\n';;

See SL.io.3 for details.

Initialize variables before use

The mod variable is printed before it is set to any value.  That is not likely to be useful.  Better is to always make sure that all variables are initialized before using them.

Use constant string concatenation

This code currently includes these lines:

cout << "U kojem modu kalkulatora zelis biti? \n" << mod;
cout << "Mod 1 je za zbrajanje \n";
cout << "Mod 2 je za oduzimanje \n";
cout << "Mod 3 je za mnozenje \n";
cout << "Mod 4 je za dijeljenje \n";

I would suggest writing it instead like this:

std::cout <<
    "U kojem modu kalkulatora zelis biti? \n"
    "Mod 1 je za zbrajanje \n"
    "Mod 2 je za oduzimanje \n"
    "Mod 3 je za mnozenje \n"
    "Mod 4 je za dijeljenje \n";

Since the next line happens to be scanf, I'd suggest instead using your prompt function.

Consider using more idiomatic C++

While your use of and and not is not techncially wrong, you should be aware that many experienced C++ programmers will be unfamiliar with these operator alternatives, and so if others read your code, it might be an impediment to their understanding of the code.

Eliminate redundant variables

I don't speak Croatian, but the variable names are good in that they accurately describe what the variables mean in the context of the program.  However, I would suggest that instead of zbroj, razlika, umnozak, kolicnik you could use a single variable rezultat.

Don't Repeat Yourself (DRY)

The mathematical operations in the case statement all include very similar repeated code.  Instead of repeating code, it's generally better to make common code into a function.  The only thing that varies is the prompt, the mathematical operation and the way the result is printed.  An advanced technique would be to gther them all into a data structure.  If you're just beginning, don't worry if you can't understand this yet.  It uses lambda expressions and C++17 std::string_view among other things.  I am putting it here to show how powerful C++ can be:

#include <cmath>
#include <iostream>
#include <string_view>
#include <array>

int prompt(const char* name)
{
    std::cout << name;
    int value;
    std::cin >> value;
    return value;
}

int main()
{

    struct MathOp {
        std::string_view opname;
        int (*operate)(int a, int b);
        std::string_view opsymbol;

        void operator()() const {
            std::cout << "Odabrali ste " << opname << "!\n";
            int a = prompt("a: ");
            int b = prompt("b: ");
            int rezultat = operate(a, b);
            std::cout << a << opsymbol << b << " = " << rezultat << '\n';
        }

    };

    constexpr std::array<MathOp, 4> op{{
        { "zbrajanje", [](int a, int b){ return a+b; }, " + " },
        { "oduzimanje", [](int a, int b){ return a-b; }, " - " },
        { "mnozenje", [](int a, int b){ return a*b; }, " * " },
        { "dijeljenje", [](int a, int b){
                if (b == 0) {
                    std::cout << "dijeljenje s nulom \n";
                    return 0;
                }
                return a/b;
            }, " / " },
    }};

    std::cout << "U kojem modu kalkulatora zelis biti? \n";
    for (std::size_t i{0}; i < op.size(); ++i) {
        std::cout << "Mod " << i+1 << " je za " << op[i].opname << "\n";
    }
    unsigned mod;
    std::cin >> mod;

    if (mod > 0 && mod <= op.size()) {
        op[mod-1]();
    } else {
        std::cout << "Nemamo tu opciju trenutno, prcekajte za nadogradnju programa, ili se javite developeru! \n";
    }
}

Now if we want to add exponentiation, for example, all we need to do is add a single line to the op array:

{ "eksponenciju", [](int a, int b){ return static_cast<int>(std::pow(a,b)); }, " ** " },


Calculate distance between points and price per area in Pandas I'm trying to find distances between all points (latitude, longitude), and for each point, get the average price_area (price/area) of the closest points around it. This code is taking too long:

def create_pa_radius(df, radius):

    df['pa_' + str(radius)] = np.nan

    for index, row in df.iterrows():
        point = [row['latitude'], row['longitude']]
        df['distances'] = df.apply(lambda x: geo_dist(point, [x['latitude'], x['longitude']]).km, axis = 1)
        samples = df.price_area[df.distances < radius/1000]
        mean = samples.mean()

        df['pa_' + str(radius)].iloc[index] = mean

    return df

I would like at least to understand how to make this kind of iteration faster.

The problem you are having is most likely due to your code being O(n²) on the size of your dataframe. (It's possible you have a small df and a really bad implementation of geo_dist, but I'll ignore that.)

Let's strip out most of your code:

for index, row in df.iterrows():
    df['distances'] = df.apply(lambda x: geo_dist(point, [x['latitude'], x['longitude']]).km, axis = 1)

These lines can be rewritten:

for index, row in df.iterrows():
    for i, r2 in df.iterrows():
        p2 = (r2['latitude'], r2['longitude'])
        df.distances[i] = geo_dist(point, *p2).km

The same is true for the filtering of distances < radius/1000: you are looping over all n rows, and then for each row you are looping over all n rows again. This is n * n operations, or O(n²).

There is no simple way to fix this. If you insist on computing the distances from each point to its surrounding points, you will have to structure your code this way. However, there are some alternatives:

You could use a kd-tree or a space partitioning hash of your own design to access your points. This would make pandas secondary in your access, requiring you to write more python code yourself.
You could take advantage of some other heuristic to determine proximity, and use this to filter your data before running your n² algorithm on it: note that 3² + 3² = 18, while (3 + 3)² = 36. Breaking your data down into smaller units can have a substantial impact on your performance. For example: in the United States, zip codes within a given state all start with a few identical 2 digit prefixes. You might build a dictionary of "adjoining" states, by hand, and then pre-filter your locations using a zip-code prefix, such that you only consider other points that have the same 2-digit prefixes, or the 2-digit prefixes of an adjacent state. If your locations were US cities, this would let you eliminate more than 90% of your data before you start your n² algorithm.
There are surely some other approaches, which will tend to vary based on your application. Perhaps if you mention what problem you are trying to solve, someone will have a suggestion.


Downloading stock information from Yahoo! Finance The program downloads stock information from Yahoo! Finance and displays it in the spreadsheet.  On my Mac the program takes 10 minutes to get data for approximately 4000 stocks and on the PC it takes 45 minutes.  I wrote another version which copies and pastes the data instead of iterating through each price value, but every time I run that macro, the runtime increases.  That version takes about 15 minutes on a PC and 45 minutes on a Mac.

I don't know what is going on but I just want a program that runs in < 15 minutes on both operating systems and has a consistent runtime.

Option Explicit

Sub GetData()

Application.Calculation = xlCalculationManual

Dim numb_tickers As Integer
numb_tickers =        Application.WorksheetFunction.CountBlank(Worksheets("Sheet1").Range(Sheet1.Cells(2, 2), Sheet1.Cells(5000, 2)))
numb_tickers = 5000 - numb_tickers

Dim start_ticker As Integer
start_ticker = Sheet3.Cells(5, 2) + 1
Dim end_ticker As Integer
end_ticker = Sheet3.Cells(6, 2) + 1

Dim x As Integer
For x = start_ticker To end_ticker

    Dim DataSheet As Worksheet
    Dim EndDate As Date
    Dim StartDate As Date
    Dim Symbol As String
    Dim qurl As String
    Dim nQuery As Name
    Dim LastRow As Integer

    Application.ScreenUpdating = False
    Application.DisplayAlerts = False

    Sheets("Data").Cells.Clear

    Set DataSheet = ActiveSheet

        StartDate = Sheet3.Cells(2, 2)
        EndDate = Sheet3.Cells(3, 2)
        Symbol = Sheet1.Cells(x, 2)
        Sheets("Data").Range("a1").CurrentRegion.ClearContents

        qurl = "http://ichart.finance.yahoo.com/table.csv?s=" & Symbol
        qurl = qurl & "&a=" & Month(StartDate) - 1 & "&b=" & Day(StartDate) & _
            "&c=" & Year(StartDate) & "&d=" & Month(EndDate) - 1 & "&e=" & _
            Day(EndDate) & "&f=" & Year(EndDate) & "&g=" & Sheets("Data").Range("a1") & "&q=q&y=0&z=" & _
            Symbol & "&x=.csv"

QueryQuote:
             With Sheets("Data").QueryTables.Add(Connection:="URL;" & qurl, Destination:=Sheets("Data").Range("a1"))
                .BackgroundQuery = True
                .TablesOnlyFromHTML = False
                On Error GoTo ErrorHandler
                .Refresh BackgroundQuery:=False
                .SaveData = True
            End With

            Sheets("Data").Range("a1").CurrentRegion.TextToColumns Destination:=Sheets("Data").Range("a1"), DataType:=xlDelimited, _
                TextQualifier:=xlDoubleQuote, ConsecutiveDelimiter:=False, Tab:=True, _
                Semicolon:=False, Comma:=True, Space:=False, other:=False

         Sheets("Data").Columns("A:G").ColumnWidth = 12

Dim numb_days As Integer
numb_days = Application.WorksheetFunction.CountBlank(Worksheets("Data").Range(Sheet2.Cells(2, 7), Sheet2.Cells(1000, 7)))
numb_days = 1000 - numb_days

Dim z As Integer
For z = 2 To numb_days + 1

    Worksheets("Sheet1").Cells(x, z + 5) = Worksheets("Data").Cells(z, 7)

Next z

ErrorHandler:  Resume Next

Next x

Dim xyz As Integer
For xyz = 2 To numb_days + 1
    Worksheets("Sheet1").Cells(1, xyz + 5) = Worksheets("Data").Cells(xyz, 1)
Next xyz

Application.Calculation = xlCalculationAutomatic

End Sub

I don't know what would cause the large difference in time, but I found a couple minor factors:

Application.ScreenUpdating = False is performed in each loop (~4 000 times I assume)
Application.DisplayAlerts = False is not only performed in the same volume but also not negated in the scope of this question.
The lack of .Value2 has a potential performance demerit as discussed here.


doCrypto method for asymetric key (RSA) I'm implementing rsa and aes classes in java, but i have some trouble with the rsa implementation.
Since i have two keys in asymetric encryption, with two different types, i've made two differents methods, one for each. Is there a better/cleaner way to do that?

private static byte[] doCrypto(
        int cipherMode,
        PublicKey publicKey,
        byte[] inputBytes)
        throws CryptoException {
    Cipher cipher;
    byte[] outputBytes;
    try {
        cipher = Cipher.getInstance("RSA");
        cipher.init(cipherMode, publicKey);
        outputBytes = cipher.doFinal(inputBytes);
    } catch (NoSuchPaddingException
            | NoSuchAlgorithmException
            | InvalidKeyException
            | BadPaddingException
            | IllegalBlockSizeException ex) {
        throw new CryptoException("Error doCrypto", ex);
    }
    return outputBytes;
}

private static byte[] doCrypto(
        int cipherMode,
        PrivateKey privateKey,
        byte[] inputBytes)
        throws CryptoException {
    Cipher cipher;
    byte[] outputBytes;
    try {
        cipher = Cipher.getInstance("RSA");
        cipher.init(cipherMode, privateKey);
        outputBytes = cipher.doFinal(inputBytes);
    } catch (NoSuchPaddingException
            | NoSuchAlgorithmException
            | InvalidKeyException
            | BadPaddingException
            | IllegalBlockSizeException ex) {
        throw new CryptoException("Error doCrypto", ex);
    }
    return outputBytes;
}

Regarding the two different methods for the encrypt/decrypt, yes, you can reduce them to just one function. The PrivateKey and PublicKey classes both extend the Key interface, and that's what's needed for the init(...) function.

Note also, that you are doing some code-style things "wrong". Your try/catch code should be using the more modern try-with-resources mechanism, and there's no need to declare the byte[] array outside the try block.

So, reducing it all down to a single method, with improved code style, it would be:

private static byte[] doCrypto(int cipherMode,  Key key, byte[] inputBytes)
          throws CryptoException {

    try (Cipher cipher = Cipher.getInstance("RSA")) {
        cipher.init(cipherMode, key);
        return cipher.doFinal(inputBytes);
    } catch (NoSuchPaddingException
            | NoSuchAlgorithmException
            | InvalidKeyException
            | BadPaddingException
            | IllegalBlockSizeException ex) {
        throw new CryptoException("Error doCrypto", ex);
    }
}


Constraint Programming: Map color problem I've written some python code to solve the map coloring problem. In my code, I represent the problem using Territory and MapColor objects:

class Territory:

    def __init__(self, name, neighbors, color = None):

        self.name      = name
        self.neighbors = neighbors
        self.color     = color

    def __str__(self):
        return str((self.name, self.neighbors, self.color))

class MapColor:

    def __init__(self, graph, colors):

        self.map     = graph
        self.colors  = colors
        self.vars    = list(self.map.keys())
        self.domains = { var: set(self.colors) for var in self.vars }

Here, MapColor.map represents a {str: Territory} mapping and MapColor.vars, the string keys of the the dictionary (my solve function visits the territories in the dictionary iteratively; once we've exhausted this list, we have know the problem has been solved). Here's an example how this problem can be instantiated:

WA  = 'western australia'
NT  = 'northwest territories'
SA  = 'southern australia'
Q   = 'queensland'
NSW = 'new south wales'
V   = 'victoria'
T   = 'tasmania'

colors    = {'r', 'g', 'b'}

australia = { T:   Territory(T,   [V]                 ),
              WA:  Territory(WA,  [NT, SA]            ),
              NT:  Territory(NT,  [WA, Q, SA]         ),
              SA:  Territory(SA,  [WA, NT, Q, NSW, V] ),
              Q:   Territory(Q,   [NT, SA, NSW]       ),
              NSW: Territory(NSW, [Q, SA, V]          ),
              V:   Territory(V,   [SA, NSW, T]        ) }

problem = MapColor(australia, colors)

Notice that MapColor.domains is used to keep track of the possible colors a Territory can take. My solve function in fact adds some optimization to the standard backtracking method by reducing the domains of neighboring territories. In this function, the variable i is used to iterate through the keys of MapColor.map.

As I write in the comments, this code adapts the approach typically used to solve puzzles: the idea is to make a move and then continue solving the rest of the puzzle, moving to the next open spot on the board. Since we can't iterate over a "board" with explicit indices, I iterate over the keys of MapColor.map.

def solve(self, i):

    if i == len(self.vars):
        return True

    # discussion on why we keep track of old domain is in comments
    old = {var: set(self.domains[var]) for var in self.vars}
    var = self.vars[i]
    print 'domain for '  + var + ': ' + str(self.domains[var])
    if self.map[var].color != None:
        return self.solve(i + 1)

    for color in self.domains[var]:

        if self.is_valid(var, color):

            self.set_map(var, color)
            self.remove_from_domains(var, color)

            if self.solve(i + 1):
                return True

            self.set_map(var, None)
            self.domains = old

    return False

My question: How can I use a heap (python has heapq) to choose to visit the Territory with the smallest domain next? Should I pass the heap along with each recursive call?

Or should I try to make this method iterative? Would that require using both a stack and a heap? Advice on how to do this would be much appreciated!

For reference, here are the other functions used in my code:

def is_valid(self, var, color):
    territory = self.map[var]

    for neighbor in territory.neighbors:
        if color == self.map[neighbor].color:
            return False

    return True

def set_map(self, key, color):
    self.map[key].color = color

def remove_from_domains(self, key, color):
    territory = self.map[key]

    for var in territory.neighbors:
        if self.map[var].color != None:
            continue
        if color in self.domains[var]:
            self.domains[var].remove(color)

Your code looks good but I think you should have a look at PEP 8 which is the Style Guide for Python Code. Among other things, it says :

  Comparisons to singletons like None should always be done with is or is not, never the equality operators.

Also, you should probably add documentation especially as the solve method takes a parameter for no obvious reason. PEP 257 describes the Docstring Conventions.

Finally, the name you give to your variables and members can be quite confusing.

Your Territory class describes a territory by its name, its neighbours and its colors. The association from territory to color is valid only in the context of a MapColor. Thus, Territories could be described just by their name and neighbours. Instead of defining Australia with :

australia = { T:   Territory(T,   [V]                 ),
              WA:  Territory(WA,  [NT, SA]            ),
              NT:  Territory(NT,  [WA, Q, SA]         ),
              SA:  Territory(SA,  [WA, NT, Q, NSW, V] ),
              Q:   Territory(Q,   [NT, SA, NSW]       ),
              NSW: Territory(NSW, [Q, SA, V]          ),
              V:   Territory(V,   [SA, NSW, T]        ) }

one could get rid of the Territory class alltogether and remove the duplicated values by defining a mapping from name to containers of neighbours. Because neighbours do not need to be in a given order, a set will do the trick :

australia = { T:   {V                },
              WA:  {NT, SA           },
              NT:  {WA, Q, SA        },
              SA:  {WA, NT, Q, NSW, V},
              Q:   {NT, SA, NSW      },
              NSW: {Q, SA, V         },
              V:   {SA, NSW, T       } }

I also took this chance to add a function to check that the graph is properly defined :

def check_valid(graph):
    for node,nexts in graph.iteritems():
        assert(nexts) # no isolated node
        assert(node not in nexts) # # no node linked to itself
        for next in nexts:
            assert(next in graph and node in graph[next]) # A linked to B implies B linked to A

Then, in the MapColor class, you can add a dictionnary to map nodes to their colors (if any).

You don't need to store colors in your MapColor class as you don't reuse it afterward.
Also, the nodes member is not required neither.

The is_valid method can be rewritten in a more concise way using the function all.

The biggest problem in your code is probably the fact that most interesting function takes a parameter for no obvious reason. Easiest solution would be to make it a default parameter with value 0. A more interesting solution would be to try to understand when we want to stop which is when all nodes have been given a color. In order to do so, get the list of nodes with no color and consider we have a valid solution if this list is empty :

   uncolored_nodes = [n for n,c in self.node_colors.iteritems() if c is None]
    if not uncolored_nodes:
        print self.node_colors
        return True

Then, in order to know what is the next node to consider, just take the first of the list (at this stage, we know that the list cannot be empty):

    node = uncolored_nodes[0]

(This also makes obvious the fact that condition if self.map[var].color != None: (which would be written self.node_colors[node] is not None in my code) cannot be true by definition of node.

Once comments have been taken into account (except for the comments because I can't be bothered), the code looks like :

#!/usr/bin/python

def check_valid(graph):
    for node,nexts in graph.iteritems():
        assert(nexts) # no isolated node
        assert(node not in nexts) # # no node linked to itself
        for next in nexts:
            assert(next in graph and node in graph[next]) # A linked to B implies B linked to A

class MapColor:

    def __init__(self, graph, colors):
        check_valid(graph)
        self.graph = graph
        nodes = list(self.graph.keys())
        self.node_colors  = { node: None for node in nodes }
        self.domains = { node: set(colors) for node in nodes }

    def solve(self):
        uncolored_nodes = [n for n,c in self.node_colors.iteritems() if c is None]
        if not uncolored_nodes:
            print self.node_colors
            return True

        node = uncolored_nodes[0]
        print 'domain for '  + node + ': ' + str(self.domains[node])
        for color in self.domains[node]:
            if all(color != self.node_colors[n] for n in self.graph[node]):
                self.set_color(node, color)
                self.remove_from_domains(node, color)

                if self.solve():
                    return True

                self.set_color(node, None)
                self.add_to_domains(node, color)

        return False

    def set_color(self, key, color):
        self.node_colors[key] = color

    def remove_from_domains(self, key, color):
        for node in self.graph[key]:
            if color in self.domains[node]:
                self.domains[node].remove(color)

    def add_to_domains(self, key, color):
        for node in self.graph[key]:
            self.domains[node].add(color)

WA  = 'western australia'
NT  = 'northwest territories'
SA  = 'southern australia'
Q   = 'queensland'
NSW = 'new south wales'
V   = 'victoria'
T   = 'tasmania'

colors    = {'r', 'g', 'b'}

australia = { T:   {V                },
              WA:  {NT, SA           },
              NT:  {WA, Q, SA        },
              SA:  {WA, NT, Q, NSW, V},
              Q:   {NT, SA, NSW      },
              NSW: {Q, SA, V         },
              V:   {SA, NSW, T       } }

problem = MapColor(australia, colors)

problem.solve()

Once all of this has been done, one can notice that the node_colors and domains contain somewhat duplicated information : a node n has color c if and only if domains[n] == {c}. I have no time to get rid of node_colors but that would be my next suggestion. Also, there might be a problem with the way you add/remove colors because when calling self.add_to_domains(node, color) we might re-add a color that has been removed at an earlier stage. I am not able to find an example right now but it might be something to consider. Please let me know if you think I am wrong.

Problem in your code (and in mine) : trying to make the test case a bit more interesting (at the moment, your example does not encounter any problem), I've discovered a problem.

Indeed, I was adding Queensland as a neighboor of Victoria (which is still possible in a planar graph if the Sunshine State decided to invade the West Coast of NSW) and I got the following error :

Traceback (most recent call last):
  File "./colors_original.py", line 99, in <module>
    problem.solve(0)
  File "./colors_original.py", line 43, in solve
    if self.solve(i + 1):
  File "./colors_original.py", line 43, in solve
    if self.solve(i + 1):
  File "./colors_original.py", line 36, in solve
    for color in self.domains[var]:
RuntimeError: Set changed size during iteration

Before trying to go for better perf, I think you should try to make this correct. Because maintaining a consistent state whilst calling recursive method can be quite hard, I guess you should try to make things as simple as possible. I'll try to update this answer if I think of anything interesting but it is quite unlikely.


Double series sequences How can I shorten this class method (currently 10 lines long)?

class DoubleSeries
  def self.generate_sequences(seq1_start, seq2_start, seq1_step, seq2_step, iterations)
    seq1,seq2=seq1_start,seq2_start
    result=[]
    (1..iterations).each do
      result << seq1 << seq2
      seq1+=seq1_step
      seq2+=seq2_step
    end
    result
  end
end

I tried doing:

class DoubleSeries
  def self.generate_sequences(seq1_start, seq2_start, seq1_step, seq2_step, iterations)
    seq1,seq2=seq1_start,seq2_start
    result=
    (1..iterations).each do
      self << seq1 << seq2
      seq1+=seq1_step
      seq2+=seq2_step
    end
    result
  end
end

but self is the class at this point (as this is a class method), not the result= variable.

You could a number of different things. For instance, using ranges and a couple of Array methods, I get this:

class DoubleSeries
  def self.generate_sequences(seq1_start, seq2_start, seq1_step, seq2_step, iterations)
    seq1_end = seq1_start + iterations * seq1_step
    seq2_end = seq2_start + iterations * seq2_step
    seq1 = (seq1_start..seq1_end).step(seq1_step)
    seq2 = (seq2_start..seq2_end).step(seq1_step)
    seq1.zip(seq2)
  end
end

There's still some nasty repetition going on. I might suggest simply skipping all this in favor of a generate_sequence method, that only returns a single sequence. Then you can use that plus Array#zip to generate double, triple, or whatever sequences:

def generate_sequence(start, step, iterations)
  limit = start + iterations * step
  (start..limit).step(step).to_a
end

double_seq = generate_sequence(a, b, c).zip(generate_sequence(x, y, z))


Checking if an array is unimodal Codeforces problem 831A says:

Array of integers is unimodal, if:

it is strictly increasing in the beginning;
after that it is constant;
after that it is strictly decreasing.

The first block (increasing) and the last block (decreasing) may be absent. It is allowed that both of this blocks are absent.
For example, the following three arrays are unimodal: [5, 7, 11, 11, 2, 1], [4, 4, 2], [7], but the following three are not unimodal: [5, 5, 6, 6, 1], [1, 2, 1, 2], [4, 5, 5, 6].
Write a program that checks if an array is unimodal.

I solved this using two methods in python3.5.
I was expecting method 2 to be faster than method 1 but auto-grader showed the opposite.
So, I wanted to know the reason that what makes the difference here?
Method 1
a, lst = int(input()), input().split()

lst = [int(ch) for ch in lst]

def f(m, n, flag):
    if flag == 1:
        return m < n
    elif flag == 2:
        return m == n
    else:
        return m > n

def is_modal(a, lst):
    flag = 1
    for i in range(1, a):
        if f(lst[i - 1], lst[i], flag):
            continue
        else:
            if flag == 3:
                return "NO"
            flag += 1
            if f(lst[i - 1], lst[i], flag):
                continue
            else:
                if flag == 3:
                    return "NO"
                flag += 1
                if f(lst[i - 1], lst[i], flag):
                    continue
                else:
                    return "NO"
    return "YES"

print(is_modal(a, lst))

It seems to be overly complicated to me, at first I solved it in this way but I did not like it so I tried another way, hence came at method 2.
Method 2
a, lst = int(input()), input().split()
lst = [int(ch) for ch in lst]

def is_modal(a, lst):
    i = 1
    for i in range(i, a):
        if not (lst[i - 1] < lst[i]):
            break
    else:
        return "YES"

    for i in range(i, a):
        if not (lst[i - 1] == lst[i]):
            break
    else:
        return "YES"

    for i in range(i, a):
        if not (lst[i - 1] > lst[i]):
            return "NO"
    else:
        return "YES"

print(is_modal(a, lst))

It looks cleaner and simpler but still, it came out to be slower one.
And apart from comparing these codes, any other suggestions are also welcome.

The following code is similar to your method 2:

def is_unimodal(seq):
    i = 1
    end = len(seq)
    while i < end and seq[i - 1] < seq[i]:
        i += 1
    while i < end and seq[i - 1] == seq[i]:
        i += 1
    while i < end and seq[i - 1] > seq[i]:
        i += 1
    return i == end

This code probably looks more like C than Python, but I expect it to be fast and readable.


Nested loop with synchronous and asynchronous behavior The problem is as follows. I have an array of objects like so:

let myObj = [
{'db1':['doc1','doc2','doc3']},
{'db2':['doc4','doc5']},
{'db3':['doc7','doc8','doc9','doc10']}
]

Note that this is a data structure I decided to use for the problem and can be changed if it can improve the overall implementation. The actual db and document ids are read from a text file formatted as below.

"db1","doc1"
"db1","doc2"
...

My app will iterate through the db list synchronously. Inside each db iteration, there will be an asynchronous iteration of the document list. Each document will be retrieved, processed and saved back to the db.

So basically at any given instance: one db, but multiple documents.

I have a working implementation of the above like so:

dbIterator: the synchronous outer loop to iterate dbs. The callback passed to docIterator will trigger the next iteration.

const dbIterator = function (x) {
  if (x < myObj.length) {
    let dbObj = myObj[x];
    let dbId = Object.keys(dbObj)[0];
    docIterator(dbId, dbObj[dbId], ()=>dbIterator(x+1));
  } else {
    logger.info('All dbs processed');
  }
};

docIterator: the asynchronous loop to iterate docs. The callback cb is called after all documents are processed. This is tracked via the docsProcessed and docsToBeProcessed variables

const docIterator = function(dbId, docIds, cb){
  //create connection
  targetConnection = //some config for connection to dbId
  let docsProcessed = 0;
  let docsToBeProcessed = docIds.length;

  //asynchronous iteration of documents
  docIds.forEach((docId)=>{
    getDocument(docId, targetConnection).then((doc)=>{
      //process document
      processDoc(doc, targetConnection).then(()=>{
        //if processing is successful
        if (++docsProcessed >= docsToBeProcessed) {
          cb();
        }
      })
       //if processing fails
      .catch((e) => {
        logger.error('error when processing document');
        if (++docsProcessed >= docsToBeProcessed) {
          cb();
        }
      });

    }).catch((e)=>{
      logger.error('error when retrieving document: ');
      if (++docsProcessed >= docsToBeProcessed) {
        cb();
      }
    });
  });
};

processDoc: used to process and save an individual document. This returns a promise that gets resolved when the document processing is done which in turn increments docsProcessed and conditionally (docsProcessed >= docsToBeProcessed) calls the call back passed into docIterator

const processDoc = function(doc, targetConnection) {

  return new Promise(function(resolve, reject) {
    if(shouldThisDocBeProcessed(doc){
      let updatedDoc = logic(doc);
      targetConnection.insert(updatedDoc, updatedDoc._id,
        function (error, response) {
          if (!error){
            logger.info('updated successfully');
          } else {
            logger.error('error when saving doc');
          }
          resolve();
        }
      );
    } else {
      resolve();
    }
  })
};

This works as expected but for me this implementation is sub-optimal and messy. I'm pretty sure this can be improved upon and most importantly a chance to better understand and implement solutions to synchronous and asynchronous problems.

I'm open to constructive criticism. So how can this be improved?

First you can make the database container more precise and accessible if you define it like this:

var dbs = [
    { id: 'db1', docs: ['doc1', 'doc2', 'doc3'] },
    { id: 'db2', docs: ['doc4', 'doc5', 'doc6'] },
];

So you can access the database id via db.id and don't need to calculate it with dbId = Object.keys(dbObj)[0]

Second the function's names aren't so good. The names dbIterator, docIterator and merchantIterator tell about HOW they do their work, but they should better tell WHAT they do as it is the case with getDocument.

Third in docIterator you are nesting promises but they are meant for chaining.

Fourth in docIterator the checks if (++docsProcessed >= docsToBeProcessed) are needless because forEach makes sure that all documents will be processed and it will stop if there are no more.

When you are dealing with loops and asynchronous functions you should have a look at the Promise.all method. Provided that merchantIterator does realy nothing more than triggering the next iteration (as you suggested) I think the processing of all databases and documents may look like this:

function processDatabases(dbs) {
    Promise.all(dbs.map(db => {
        var connection = // some config for connection to **db.id**
        return Promise.all(db.docs.map(doc => {
            return getDocument(doc, connection)
                .then(doc => processDoc(doc, connection))
                .catch(err => logger.error(err.message));
        }));
    }))
    .then(() => logger.info('all done'));
}

In this case getDocument and processDoc should reject or throw an Error with an appropriate message.


PyQt 5 CheckBox State Efficient wAy Firstly, have a good day to all,
I've been trying to learn PyQt5 nowadays. I wrote a code about check box state changing depends on other check box. It looks work. I wonder your feed back.
Thank you for your time.

from PyQt5.QtWidgets import *
from PyQt5.QtCore import *
import sys

class CheckBox(QWidget):

    def __init__(self):
        super(CheckBox, self).__init__()
        self.features()
        self.checkbox()

    def features(self):
        self.setWindowTitle("Check Box")
        self.setGeometry(500, 200, 200, 200)

        self.show()

    def checkbox(self):
        # Check Box
        self.check_box1 = QCheckBox("Alive")
        self.check_box2 = QCheckBox("Dead")
        self.check_box3 = QCheckBox("No Idea")
        # Check Box State
        self.check_box3.setChecked(True)
        self.check_box1.stateChanged.connect(self.onStateChange)
        self.check_box2.stateChanged.connect(self.onStateChange)
        self.check_box2.stateChanged.connect(self.onStateChange)
        # Button
        self.push_button1 = QPushButton("Send")
        self.push_button1.clicked.connect(lambda: self.sendmessage(self.check_box1.isChecked(),
                                                                self.check_box2.isChecked(),
                                                           self.check_box3.isChecked(), self.label1))
        # Label
        self.label1 = QLabel("")
        # Vertical Lay Out
        v_box1 = QVBoxLayout()
        v_box1.addWidget(self.check_box1)
        v_box1.addWidget(self.check_box2)
        v_box1.addWidget(self.check_box3)
        v_box1.addWidget(self.push_button1)
        v_box1.addWidget(self.label1)
        # Horizontal Lay Out
        h_box1 = QHBoxLayout()
        h_box1.addStretch()
        h_box1.addLayout(v_box1)
        h_box1.addStretch()
        # Set Lay Out
        self.setLayout(h_box1)

    def onStateChange(self, state):
        if state == Qt.Checked:
            if self.sender() == self.check_box1:
                self.check_box2.setChecked(False)
                self.check_box3.setChecked(False)
            elif self.sender() == self.check_box2:
                self.check_box1.setChecked(False)
                self.check_box3.setChecked(False)
            elif self.sender() == self.check_box3:
                self.check_box1.setChecked(False)
                self.check_box2.setChecked(False)

    def sendmessage(self, cbox1, cbox2, cbox3, lab):
        if cbox1:
            lab.setText("Check Box 1")
        elif cbox2:
            lab.setText("Check Box 2")
        elif cbox3:
            lab.setText("Check Box 3")
        else:
            lab.setText("No Selection")

app = QApplication(sys.argv)
checkbox = CheckBox()
sys.exit(app.exec_())

I think there is a mistake, this line is defined twice (line 28):

self.check_box2.stateChanged.connect(self.onStateChange)

Therefore the event onStateChange is triggered twice when you check check_box2, which is logical.

I think you did a copy paste and the last should be check_box3. But your naming conventions are not intuitive, give your objects some more meaningful names, otherwise how are you going to tell the difference from your code.

If what you want is mutually exclusive checkboxes the implementation could be more straightforward. Personally I prefer to use radio buttons like in plain HTML because this is more intuitive (it is immediately obvious that only one answer is allowed).

First approach: a generic method that loops on the checkboxes in your form and unchecks all of them except the sender. Then you can simplify code and get rid of if/elif

Second approach: use QT built-in features. You could wrap your checkboxes in a QButtonGroup container.

  A rectangular box before the text label appears when a QCheckBox
  object is added to the parent window. Just as QRadioButton, it is also
  a selectable button. Its common use is in a scenario when the user is
  asked to choose one or more of the available options.

  Unlike Radio buttons, check boxes are not mutually exclusive by
  default. In order to restrict the choice to one of the available
  items, the check boxes must be added to QButtonGroup.

and:

  As mentioned earlier, checkBox buttons can be made mutually exclusive
  by adding them in the QButtonGroup object.

self.bg = QButtonGroup()
self.bg.addButton(self.b1,1)
self.bg.addButton(self.b2,2)

  QButtonGroup object, provides abstract container for buttons and
  doesn’t have a visual representation. It emits buttonCliked() signal
  and sends Button object’s reference to the slot function btngroup().

Source: PyQt - QCheckBox Widget


Python 'interest rate' object I have only recently started learning python and the below is my attempt to create an 'interest rate' object which, given one of several types of interest/discount rate, gives the user access to all of the other corresponding rates that are equivalent to this. This looks as follows:

import math

class InterestRate:
    def __init__(self, effectiveInterestRate=0, nominalInterestRate=0, effectiveDiscountRate=0, nominalDiscountRate=0, discountFactor=0, forceOfInterest=0, paymentsPerPeriod=1):

        self.effectiveInterestRate = effectiveInterestRate
        self.nominalInterestRate = nominalInterestRate
        self.effectiveDiscountRate = effectiveDiscountRate
        self.nominalDiscountRate = nominalDiscountRate
        self.discountFactor = discountFactor
        self.forceOfInterest = forceOfInterest
        self.paymentsPerPeriod = paymentsPerPeriod

        numberOfRatesProvided = 0
        for attr, value in vars(self).items():
            if value != 0 and attr != 'paymentsPerPeriod':
                numberOfRatesProvided += 1
                rateGiven = attr
        if numberOfRatesProvided != 1:
            raise Exception("Incorrect number of inputs passed to InterestRate object")

        if rateGiven == 'nominalInterestRate': self.effectiveInterestRate = (nominalInterestRate/paymentsPerPeriod + 1)**paymentsPerPeriod - 1
        elif rateGiven == 'effectiveDiscountRate': self.effectiveInterestRate = effectiveDiscountRate / (1 - effectiveDiscountRate)
        elif rateGiven == 'nominalDiscountRate': self.effectiveInterestRate = (1 - nominalDiscountRate/paymentsPerPeriod)**-paymentsPerPeriod - 1
        elif rateGiven == 'discountFactor': self.effectiveInterestRate = 1/discountFactor - 1
        elif rateGiven == 'forceOfInterest': self.effectiveInterestRate = math.exp(forceOfInterest) - 1

        self.nominalInterestRate = self.paymentsPerPeriod*((1+self.effectiveInterestRate)**(1/self.paymentsPerPeriod) - 1)
        self.effectiveDiscountRate = self.effectiveInterestRate/(1+self.effectiveInterestRate)
        self.nominalDiscountRate = self.paymentsPerPeriod*(1 - (1 + self.effectiveInterestRate)**(-1/self.paymentsPerPeriod))
        self.discountFactor = 1 / (1 + self.effectiveInterestRate)
        self.forceOfInterest = math.log(1 + self.effectiveInterestRate)

    def outputInterestRateData(self):
        print("\n  ----------------------------------------------------")
        print("                   Interest Rate Data                 ")
        print("  ----------------------------------------------------")
        print("   Payments per Period:        ", self.paymentsPerPeriod)
        print("   Effective Interest Rate:    ", self.effectiveInterestRate)
        print("   Effective Discount Rate:    ", self.effectiveDiscountRate)
        print("   Nominal Interest Rate:      ", self.nominalInterestRate)
        print("   Nominal Discount Rate:      ", self.nominalDiscountRate)
        print("   Discount Factor:            ", self.discountFactor)
        print("   Force Of Interest:          ", self.forceOfInterest)
        print("  ----------------------------------------------------")

    def shiftedPaymentPV(paymentAmmount, periodsShifted):
        return paymentAmmount*(1 + self.effectiveInterestRate)**periodsShifted

Assuming that it is clear enough what I have attempted to do here, can anyone offer some constructive criticism on my first attempt at a python class module?

Trying to use the code

The class defines something which feels a bit weird to use. We'll see a few ways to improve this.

The constructor provides default value for all parameters which leads one to think he can just do r = InterestRate() and enjoy. It is most definitly not the case because one (and only one) of the inputs must be provided.

This suggests that maybe we should not have a single constructor doing many things but many small constructors (using alternative constructors for example).

Then, trying to use the shiftedPaymentPV method, I stumbled on a different error: the self is not part of the signature which makes it unusable as far as I can tell.

Simplifying the code

In the __init__, we set the paymentsPerPeriod attribute only to excluse it in next operation when handling all attributes. We may as well set it afterward to makes things more simple.

This also shows how subtle and fragile that piece of logic is. One could easily break the whole thing by moving an attribute which doesn't seem to be used because we use introspection magic (which is pretty cool in itself but probably better to avoid here).

Let's see how it can be improved anyway.

Instead of counting elements as we go, we could put them in a list and count them afterwards. This helps if we want to show the list to the user in the exception message.

    ratesGiven = [attr for attr, value in vars(self).items() if value != 0 and attr != 'paymentsPerPeriod']
    if len(ratesGiven) != 1:
        raise Exception("Incorrect number of inputs passed to InterestRate object")
    rateGiven = ratesGiven[0]

This could give use the following code:

import math

class InterestRate:

    @classmethod
    def from_nominal(cls, nominalInterestRate, paymentsPerPeriod=1):
        effectiveInterestRate = (nominalInterestRate/paymentsPerPeriod + 1) ** paymentsPerPeriod - 1
        return cls(effectiveInterestRate=effectiveInterestRate, paymentsPerPeriod=paymentsPerPeriod)

    @classmethod
    def from_effective_discount_rate(cls, effectiveDiscountRate, paymentsPerPeriod=1):
        effectiveInterestRate = effectiveDiscountRate / (1 - effectiveDiscountRate)
        return cls(effectiveInterestRate=effectiveInterestRate, paymentsPerPeriod=paymentsPerPeriod)

    @classmethod
    def from_nominal_discount_rate(cls, nominalDiscountRate, paymentsPerPeriod=1):
        effectiveInterestRate = (1 - nominalDiscountRate/paymentsPerPeriod) ** -paymentsPerPeriod - 1
        return cls(effectiveInterestRate=effectiveInterestRate, paymentsPerPeriod=paymentsPerPeriod)

    @classmethod
    def from_discount_factor(cls, discountFactor, paymentsPerPeriod=1):
        effectiveInterestRate = 1 / discountFactor - 1
        return cls(effectiveInterestRate=effectiveInterestRate, paymentsPerPeriod=paymentsPerPeriod)

    @classmethod
    def from_force_of_interest(cls, forceOfInterest, paymentsPerPeriod=1):
        effectiveInterestRate = math.exp(forceOfInterest) - 1
        return cls(effectiveInterestRate=effectiveInterestRate, paymentsPerPeriod=paymentsPerPeriod)

    @classmethod
    def from_effective_interest_rate(cls, effectiveInterestRate, paymentsPerPeriod=1):
        return cls(effectiveInterestRate=effectiveInterestRate, paymentsPerPeriod=paymentsPerPeriod)

    def __init__(self, effectiveInterestRate, paymentsPerPeriod):
        self.effectiveInterestRate = effectiveInterestRate
        self.paymentsPerPeriod = paymentsPerPeriod

        self.nominalInterestRate = paymentsPerPeriod * ((1 + effectiveInterestRate) ** (1 / paymentsPerPeriod) - 1)
        self.effectiveDiscountRate = effectiveInterestRate / (1 + effectiveInterestRate)
        self.nominalDiscountRate = paymentsPerPeriod * (1 - (1 + effectiveInterestRate) ** (-1 / paymentsPerPeriod))
        self.discountFactor = 1 / (1 + effectiveInterestRate)
        self.forceOfInterest = math.log(1 + effectiveInterestRate)

    def outputInterestRateData(self):
        print("\n  ----------------------------------------------------")
        print("                   Interest Rate Data                 ")
        print("  ----------------------------------------------------")
        print("   Payments per Period:        ", self.paymentsPerPeriod)
        print("   Effective Interest Rate:    ", self.effectiveInterestRate)
        print("   Effective Discount Rate:    ", self.effectiveDiscountRate)
        print("   Nominal Interest Rate:      ", self.nominalInterestRate)
        print("   Nominal Discount Rate:      ", self.nominalDiscountRate)
        print("   Discount Factor:            ", self.discountFactor)
        print("   Force Of Interest:          ", self.forceOfInterest)
        print("  ----------------------------------------------------")

    def shiftedPaymentPV(paymentAmmount, periodsShifted):
        assert False, "this can't work"
        return paymentAmmount*(1 + self.effectiveInterestRate)**periodsShifted

r = InterestRate.from_nominal(nominalInterestRate=-0.9375, paymentsPerPeriod=2)
print(r.outputInterestRateData())

Style

Python has a Style Guide called PEP 8. It is definitly worth reading it and trying to apply it.

In your case, that would mean adding whitespaces around operators and using the snake_case naming convention for variables and method names.

String conversions

Instead of a outputInterestRateData, maybe you could define the __str__ method.

Also, you could take this chance to use one of the various ways to format strings in Python.

You'd get something like:

    def __str__(self):
        return """
  ----------------------------------------------------
                   Interest Rate Data
  ----------------------------------------------------
   Payments per Period:     {}
   Effective Interest Rate: {}
   Effective Discount Rate: {}
   Nominal Interest Rate:   {}
   Nominal Discount Rate:   {}
   Discount Factor:         {}
   Force Of Interest:       {}
  ----------------------------------------------------
        """.format(self.paymentsPerPeriod, self.effectiveInterestRate,
                self.effectiveDiscountRate, self.nominalInterestRate,
                self.nominalDiscountRate, self.discountFactor,
                self.forceOfInterest)


Multiset implementation in Ruby I am trying to implement the Wikipedia definition of a MultiSet and need feedback on the implementation.

class MultiSet
  include Enumerable

  attr_reader :members

  def initialize enum={}
    @members = {}
    raise_error unless enum.class.include? Enumerable
    enum.each do |item|
      if @members.include?(item)
        @members[item] += 1
      else
        @members[item] = 1
      end
    end
  end

  def each(&blk)
    @members.each(&blk)
  end

  def == other
    members.to_h == other.to_h
  end

  def eql? other
    self == other
  end

  def to_h
    members.dup
  end

  def to_set
    Set.new @members.keys
  end

  def remove item
    if @members.include?(item)
      @members[item] = @members[item] - 1
      if @members[item] < 1
        @members.delete(item)
      end
    end
    self
  end

  def add item
    if @members.include?(item)
      @members[item] += 1
    else
      @members[item] = 1
    end
    self
  end

  def empty!
    @members.clear
  end

  def multiplicity item
    @members[item] == 0 ? nil : @members[item]
  end

  def include? item
    @members.include? item
  end

  def cardinality
    return 0 if @members.empty?
    @members.values.reduce(:+)
  end

  def | other
    other.each do |k,v|
      if members.include? k
        members[k] = multiplicity(k) + other.multiplicity(k)
      else
        add k
      end
    end
    self
  end

  def & other
    members.each do |k,v|
      if other.include? k
        members[k] = [multiplicity(k), other.multiplicity(k)].min
      else
        remove k
      end
    end
    self
  end

end

Sometimes you're skipping parentheses, sometimes you're not. Please be consistent
Sometimes you're accessing @members directly, sometimes you're using the members reader method. In some cases that might make sense to do one or the other, but here it's random. Again: Be consistent.
However, be careful about providing a reader. Right now, I could say a_multiset.members[some_item] = -100 or something, and things would get weird because I've messed with an internal data structure. A #members accessor might be better as simply an alias for #to_h
What is raise_error? I don't see it defined anywhere. Which, funnily enough, means that it does raise an error - an error telling you that raise_error isn't defined.
If you're going to raise an error, then raise the appropriate one:

raise ArgumentError, "enum must include the 'Enumerable' module"

enum.class.include?(Enumerable) is a roundabout way of writing enum.kind_of?(Enumerable)
Your initializer duplicates logic from you #add method; just call #add instead.
#remove can be cleaned up a little by postfixing the if members[item] < 1, just to get rid of the pyramid indentation.
Your #empty! method should probably be called #clear. That's the conventional name used by Hash, Array and Set (as you can tell, since that's what your method calls on the @members hash)
Pretty sure that your #multiplicity method is exactly backwards. You're returning nil if an item's count is zero. I think you want the exact opposite: Return zero if the item doesn't exist:

include?(item) ? members[item] : 0

Use your own methods when you can. You have an #include? method already, so there's no need to use @members.include? all over the place.
#cardinality can be written as just

members.values.reduce(0, :+)

If you provide an initial value to #reduce (the zero), you don't need the extra emptiness-check you have now
Pretty sure your union method is incorrect too. If a key doesn't exist, you simply add it - meaning it'll have a count of 1. But in fact, it should have the same count as it has in the other set.
And you can simplify the method, too:

def |(other)
  other.each do |key, count|
    members[key] = multiplicity(key) + count
  end
  self
end

Both the union and intersection methods should probably return a new MultiSet instance, rather than modify the receiver.


Speeding up string splitting in Rcpp/Cpp I'm looking to speed up an Rcpp function I have written.

The function takes in a string called vcfield, which takes the format od x:y:z. This string has 3 fields if you separate it by the :. It takes an int DSfield which tells us the index (0 based) field from vcfield which is the dosage. Dosage is always one double value.

It also takes in 2 ints called aa and bb

The program then returns the value of abs[(aa + bb) - dosage].

So calling ReturnUncertainty("1/1:1.88:0,0.12,0.88", 1, 1, 1) should return 0.12, as  abs[(1+1) - 1.88] = 0.12

We should presume that the number of characters in the different fields of vcfield might vary. So it should be also be capable of taking a value like "1/1/1/1:1.88999:0,0.12,0.88".

#include <Rcpp.h>
#include <bits/stdc++.h>

using namespace Rcpp;
using namespace std;

// [[Rcpp::export]]
double ReturnUncertainty(String vcfield, int DSfield, int aa, int bb) {

    string stdfield = vcfield;
    vector <string> tokensMain;
    stringstream checkMain(stdfield);
    string intermediate;

    while (getline(checkMain, intermediate, ':')) {
        tokensMain.push_back(intermediate);
    }

    std::string DS = tokensMain[DSfield];
    double DSf;
    std::istringstream(DS) >> DSf;

    double aaD = static_cast<double>(aa);
    double bbD = static_cast<double>(bb);

    double genoSum = aaD + bbD;
    return abs(genoSum - DSf);
}

QUESTION: As a total newbie to C++, I was wondering whether or not I am doing anything that is very inefficient / unecessary which might be slowing my code down?

Remove unnecessary steps

for (int i = 0; i < field_number - 1; ++i)
{ /*read discard*/ }
double value;
// read the needed value now

This will make sure that it will not do unnecessary steps if the value needed is the very first one.

Do not copy around

I believe Rcpp would provide some way to access underlying char*. That can be used to construct std::string_view, or use the char* directly.

Use better standard library functions

My first candidate is famous (infamous) function called std::from_chars. It was accepted in C++17, but the support for that is only available with VC++'s standard library (I'm as surprised as you are, the reader, though getting FP parsing right is very hard). Basically it ignores all of the limitations of standard streams, from buffer structures to locales.

The second candidate would be std::strtod, or some such. Notice that std::stod is not a good alternative, because it requires copying parts of the string.

Going beyond standard

One of the good options is fmtlib. It has convenient interface, and is actively developed. Part of it is event coming to standard library, but unfortunately not the one we are interested in.

I'm not sure how faster it will get, but perhaps Boost.Spirit may be a good candidate. It will certainly be strong spirits to your compiler.

Facts

Of course, the benchmarks will tell which way is better. May be everything I've told is heresy.

Alternative implementation

#include <charconv>
#include <string_view>
#include <cmath>

double strparse(std::string_view s, int field, int a, int b) {
    double candidates[3];
    auto format = std::chars_format::fixed;
    auto start_point = s.data();
    auto end_point = s.data() + s.size();
    switch (field) {
    case 0:
        start_point = std::from_chars(start_point, end_point, candidates[0], format);
    case 1:
        start_point = std::from_chars(start_point, end_point, candidates[1], format);
    case 2:
        start_point = std::from_chars(start_point, end_point, candidates[2], format);
    }

    double ad = a;
    double bd = b;
    return std::abs(ad + bd - candidates[field]);
}

Well, here you have it. No copying around, manual loop unroll to prevent unnecessary steps, exact flags to maximize performance in standard library calls.

Godbolt link.

Unfortunately I do not have convenient access to a windows machine to test the implementation.


Function which removes a given word from a string Any improvements? Please review.

It could be faster if I use own strstr function which searches from the end (less memory to move) but I wanted to use only the standard functions.

char *removestr(char *str, const char *word)
{
    char *ptr = str;
    size_t len = strlen(word);
    while((ptr = strstr(ptr, word)))
    {
        if(isalnum(*(ptr + len)) || (str != ptr && isalnum(*(ptr -1))))
        {
            ptr += len;
        }
        else
        {
            memmove(ptr, ptr + len, strlen(ptr + len) + 1);
        }
    }
    return str;
}

I would suggest renaming your variables. str, ptr, and len don't tell us much about what they're meant to be.

size_t len = strlen(word);

Since this is never going to change (because it's abstractly a property of a const input) you might as well mark it const.

while((ptr = strstr(ptr, word)))

I would put a comment here, not least as a note that the assignment is deliberate. (Because so often a single equals sign in an if or while implies  a bug). Really you could do with a few more comments throughout to say why you're doing things: For example the if(isalnum... could be commented to explain that you only want to remove whole words.

strlen(ptr + len)

This may work fine, but it's a moderately expensive thing to do because it has to run down the length of the remaining list. I would be inclined to measure the length str at the start outside the loop, and track it across updates.

memmove(ptr, ptr + len, strlen(ptr + len) + 1);

Again this works fine, but it has to copy (quite slowly and carefully) the whole remaining string. Because it's in a loop, this becomes an \$ O(n^2) \$ function. One solution would be to only move back the string up to the next time word appears. This would mean a bit more complexity tracking the size of the gap that you're building up, but it would reduce the overall complexity of the function to \$ O(n) \$.


Set ENV variables for Ruby on Rails I've followed the steps from this video from railscasts for setting ENV variables.

This is my configuration:

config/database.yml

default: &default
  adapter: mysql2
  encoding: utf8
  pool: 5
  username: <%= ENV['DATABASE_USERNAME'] %>
  password: <%= ENV['DATABASE_PASSWORD'] %>
  socket: /var/run/mysqld/mysqld.sock

development:
  <<: *default
database: <%= ENV['DATABASE_NAME'] %>
test:
  <<: *default
database: <%= ENV['DATABASE_NAME'] %>
production:
  <<: *default
  database: <%= ENV['DATABASE_NAME'] %>
  username: <%= ENV['DATABASE_USERNAME'] %>
password: <%= ENV['DATABASE_PASSWORD'] %>

config/application.rb

config = YAML.load(File.read(File.expand_path('../application.yml', __FILE__)))
config.merge! config.fetch(Rails.env, {})
config.each do |key, value|
    ENV[key] = value.to_s unless value.kind_of? Hash
end

config/application.yml

development:

  DATABASE_USERNAME: "username goes here"
  DATABASE_PASSWORD: "password goes here"
  DATABASE_NAME: "database name goes here"

  SECRET_KEY_BASE: "secret key goes here"

test:

  DATABASE_USERNAME: "username goes here"
  DATABASE_PASSWORD: "password goes here"
  DATABASE_NAME: "database name goes here"

  SECRET_KEY_BASE: "secret key goes here"

production:

  DATABASE_USERNAME: "username goes here"
  DATABASE_PASSWORD: "password goes here"
  DATABASE_NAME: "database name goes here"

  SECRET_KEY_BASE: "secret key goes here"

Does this configuration follow best practices, or need some changes?

for what you use config/application.yml instead use config/secrets.yml. It is already implemented in Rails. Than you can fetch your key by:

Rails.application.secrets

even more, add to config/application.rb variable SECRETS = Rails.application.secrets and now you can use it:

SECRETS[:my_secret_key]

read more IN RAILS DOCS


Layout of two different styles of event labels with CSS I have two types of event labels. The HTML markup is:

    <time class="reg-day">Mo</time>
    <time class="reg-time">13:00-14:30 Uhr</time>

Regent Street

    <time class="work-month">Feb</time>
    <time class="work-day">24</time>

Chess Group

Moonshine Avenue

And the CSS the question is about is:

article {
    background-color: red;
    width: 20em;
    position:relative;
}

.reg-day {
   position: absolute;
   font-size:2.25em;
   font-weight:700;
   line-height:1.4;
   left:.5em;
}

.reg-time,
.reg-location {
   margin-left:5em;
   line-height: 1;
}

.work-month {
   -webkit-transform: rotate(270deg);
   width:3em;
   position: absolute;
   text-align:center;
   left:-1em;
   bottom:1em;
   font-weight:700;
}

.work-day {
   position: absolute;
   font-size:2.25em;
   font-weight:700;
   line-height:1.4;
   left:.75em;
}

.work-type,
.work-location {
   margin-left: 5em;
   line-height: 1;
}

I have two types of event labels and have created a CodePen here.

The first one has the day on the left, the time on the upper right, and the location on the lower right.

The second has the month upright on the left, the day and on the upper right the type of event, and the location on the lower right.

Basically I've managed to get the layout as I wanted.  Is the way I got there ok or is there a more robust and elegant one?

Especially the part of the day of week (of the first event type) and the day of month (of the second event type) covering more or less 2 height units of the two pieces of information on the right.

Day of week/day of month:

font-size: 3em;
line-height:1

Info:

font-size: 1em
line-height: 1.5;

would work.  That the info lines are the exact half of the day of week / day of month. Also i am unsure if using position absolute is the correct weapon of choice in that case. At least it prevents an misalignment of the margin-left: 5em elements. But over all the number of "magic numbers" feels a bit too high.

There's a difference between magic numbers and values derived from calculations.  A lot of your numbers aren't really magic, you just don't know the formula to get those values.  You can start by creating variables of the things you know are going to impact your values (eg. font-size, line-height, etc.).  Then you start looking for a formula you think will create your magic number.  When it works, change one of the values and see if it still works.  The width for $month is almost bulletproof, but the rest need help:

$padding: .5em;

$day-size: 2.25em;
$day-line-height: 1.5em;

$month-size: 1em;
$month-line-height: 1em;

body {
  padding: 10em;
}

time {
  outline: 1px solid;
  display: block;
  width: 3.5em;
  padding-left: $day-line-height;
  text-align: center;
  position: relative;

  .month {
     $width: $day-size * ($day-line-height / 1em);
     font-size: $month-size;
     transform: rotate(270deg);
     position: absolute;
     top: ($day-line-height - $month-line-height) / 2 + 1;
     left: ($width / $day-line-height - $month-size) * -1 + $padding;
     width: $width;
     text-align: center;
     line-height: $month-line-height;
     outline: 1px solid blue;
  }

  .day {
     font-size: $day-size;
     line-height: $day-line-height;
     outline: 1px solid red;
  }
}

http://codepen.io/cimmanon/pen/BcraL?editors=010

If you look at the markup in the pen, you'll notice that it is a little different than what you're doing:

<time datetime="2014-06-30">June 30</time>

<time datetime="2014-02-24">Feb 24</time>

<time datetime="2014-09-15">Sept 15</time>

<time datetime="2014-05-04">May 4</time>

The time element should not be treated like a span, it has special meaning.  If you don't have a valid date/time format (eg. 2014-05-04), then you need to use the datetime attribute to contain that information.

If you have two distinct times (such as your starting and ending time), then you should have two time elements:

<time datetime="2014-07-17T13:00Z">13:00</time>-<time datetime="2014-07-17T14:30Z">14:30</time>

Reference: http://www.brucelawson.co.uk/2012/best-of-time/

I don't feel that the article element is the appropriate choice for this type of information.  An article is supposed to be for content that can be removed from the surrounding content and still make sense.  An ordinary div or a table would be a better choice:

Event Information

        When
        Tomorrow

        Where
        The Library

        What
        Chess Club Meeting

If the th/caption elements are cramping your style, just hide them.  Tables don't have to look like tables.  It's a bit rough, but you get the idea:

http://codepen.io/cimmanon/pen/Daxkb


Guess the number Feedback Please I started learning Python these days and made my first code. Guess_The_Number and I would like some comments to improve or if I am on the right track. Thank you very much in advance
P.s: I am trying to create my own function.
def guess_number (answer):
    trys = 0

    while trys < 5:
        guessing = int(input("Your guessing is? "))
        if guessing == answer:
            print ("Yes!! it's Right!!")
            break
        elif guessing < answer:
            print ("Big")
        elif guessing > answer:
            print ("smaller")
        trys = trys+1

    if trys == 5:
        print("WoW, your trys are over  , try again!!")
        print("The number was ", answer)

print("Welcome to guess the number!!")
print("You have 5 trys to guess the number")
print("Good Luck!!")
print("I'm thiking in a number from 0 to 100")
print('')
import random
num_right = random.randint(0,100)
guess_number(num_right)

This might seem a bit much, but I'm reviewing this as if it were intended for production code:

black and isort can reformat this code to be more idiomatic without changing the functionality.

pylint, flake8 or both can tell you of some issues with this code:
$ pylint --disable=missing-function-docstring,missing-module-docstring q.py
************* Module q
q.py:3:0: C0303: Trailing whitespace (trailing-whitespace)
q.py:14:0: C0303: Trailing whitespace (trailing-whitespace)
q.py:6:8: R1723: Unnecessary "elif" after "break" (no-else-break)
q.py:23:0: C0413: Import "import random" should be placed at the top of the module (wrong-import-position)

------------------------------------------------------------------
Your code has been rated at 8.26/10

The plural of "try" is "tries".

trys += 1 is syntactic sugar for trys = trys + 1.

The code mixes I/O (input/print) and logic, which generally makes the code harder to maintain. It might be better to split this into two functions, one to check the answer against the right answer, and one to receive input and print output.

To enable reuse of the code I would add a main function and the idiomatic
if __name__ == "__main__":
    main()

lines at the bottom of the script to actually run it. Basically, if you just import your file into another nothing should print when importing it. Right now it's impossible to import cleanly from the script:
$ python
Python 3.9.1 (default, Dec 13 2020, 11:55:53)
[GCC 10.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import q
Welcome to guess the number!!
You have 5 trys to guess the number
Good Luck!!
I'm thiking in a number from 0 to 100

Your guessing is?


K&R Exercise 1-13. Printing histogram of word lengths (horizontal variant) I'm going through the K&R book (2nd edition, ANSI C ver.) and want to get the most from it. How does the following solution look to you? Note that, for the sake of exercise, I don't want to use techniques not introduced yet in the book. Also, I'm trying to reuse whatever code/philosophy already presented in the book.

/* Exercise 1-13. Write a program to print a histogram of the lengths of words in
 * its input. It is easy to draw the histogram with the bars horizontal; a vertical
 * orientation is more challenging.
 * */
/* Solution 1: Horizontal Bars
 * */
#include <stdio.h>

#define MAXLEN  10  /* Any word longer than this will get counted in the >MAXLEN histogram.
                     * For output formatting, the program assumes this won't be greater than 999.
                     * */
#define IN      1   /* inside a word */
#define OUT     0   /* outside a word */

int main()
{
    int c, state;
    int i, j;
    int histo[MAXLEN+1];
    int counter;

    for (i = 0; i < MAXLEN+1; ++i)
        histo[i] = 0;

    /* Perform the counting */
    state = OUT;
    counter = 0;
    while ((c = getchar()) != EOF)
        if (c == ' ' || c == '\n' || c == '\t') {
            if (state == IN && counter > 0) {
                if (counter-1 < MAXLEN)
                    ++histo[counter-1];
                else
                    ++histo[MAXLEN];
            }
            state = OUT;
            counter = 0;
        }
        else {
            ++counter;
            if (state == OUT)
                state = IN;
        }

    /* Print horizontal histogram. I'd use a function for formatting
     * with regards to max. number of digits, but functions haven't
     * been introduced yet.
     * */
    for (i = 0; i < MAXLEN; ++i) {
        if (MAXLEN < 10)
            printf(" %1d | ", i+1);
        else if (MAXLEN < 100)
            printf(" %2d |", i+1);
        else
            printf(" %3d |", i+1);
        for (j = 0; j < histo[i]; ++j)
            putchar('*');
        putchar('\n');
    }
    if (MAXLEN < 10)
        printf(">%1d |", i);
    else if (MAXLEN < 100)
        printf(">%2d |", i);
    else
        printf(">%3d |", i);
    for (j = 0; j < histo[i]; ++j)
        putchar('*');
    putchar('\n');
}

Output when run on the program code:

$ cat ch1-ex-1-13-01.c | ./ch1-ex-1-13-01
  1 |*********************************************************
  2 |****************************************************************
  3 |***************************************
  4 |************************************
  5 |**************
  6 |**********
  7 |************************
  8 |***********
  9 |*******
 10 |*********
>10 |**************

K&R C requires all variables to be declared at the start of their enclosing block.  This is a bad habit to learn: it's much safer to declare variables where they can be initialised:
/* these can already be initialised where they are declared */
int state = OUT;
int histo[MAXLEN+1] = { 0 };  /* C90 */
int counter = 0;

/* reduce scope, and never have uninitialized value */
for (int j = 0; j < histo[i]; ++j)  /* C99 */

We could remove the need to subtract 1 from the length when updating histo if we reserve the first element for long words instead of the last:
        if (state == IN && counter > 0) {
            if (counter <= MAXLEN)
                ++histo[counter];
            else
                ++histo[0];
        }

We obviously need to make a corresponding change to the printing:
for (int i = 1;  i <= MAXLEN;  ++i) {
    if (MAXLEN < 10)
        printf(" %1d | ", i);
    else if (MAXLEN < 100)
        printf(" %2d |", i);
    else
        printf(" %3d |", i);
    for (int j = 0;  j < histo[i];  ++j)
        putchar('*');
    putchar('\n');
}

We can avoid the if/else chain on MAXLEN in this loop, by choosing the formatting string in advance:
const char *format = "";
if (MAXLEN < 10) {
        format = " %1d | ";
} else if (MAXLEN < 100) {
        format = " %2d | ";
}  else {
        format = " %3d | ";
}

for (int i = 1;  i <= MAXLEN;  ++i) {
    printf(format, i);
    for (int j = 0;  j < histo[i];  ++j)
        putchar('*');
    putchar('\n');
}

That might not look like an improvement, but with a small change, we can use the same format string to insert > for the over-long words, too:
const char *format = "";
if (MAXLEN < 10) {
        format = "%c%1d | ";
} else if (MAXLEN < 100) {
        format = "%c%2d | ";
}  else {
        format = "%c%3d | ";
}

for (int i = 1;  i <= MAXLEN;  ++i) {
    printf(format, ' ', i);
    for (int j = 0;  j < histo[i];  ++j)
        putchar('*');
    putchar('\n');
}

/* over-long words */
printf(format, '>', MAXLEN);
for (int j = 0; j < histo[0]; ++j)
    putchar('*');
putchar('\n');

Going beyond the expected level of knowledge, we could even compute the necessary length, and pass it using %*d:
int width = 1;
for (int i = MAXLEN;  i >= 10;  i /= 10) {
    ++width;
}

for (int i = 1;  i <= MAXLEN;  ++i) {
    printf(" %*d | ", width, i);
    for (int j = 0;  j < histo[i];  ++j)
        putchar('*');
    putchar('\n');
}

/* over-long words */
printf(">%*d | ", width, MAXLEN);
for (int j = 0; j < histo[0]; ++j)
    putchar('*');
putchar('\n');

That allows us to have non-constant MAXLEN, which will be to our advantage in future when we make this a function.

Whilst the state machine is a good idea, it turns out we don't need to explicitly store state, because counter is always zero outside a word and non-zero once we're in one:
while ((c = getchar()) != EOF) {
    if (c == ' ' || c == '\n' || c == '\t') {
        if (counter > 0) {
            ++histo[counter <= MAXLEN ? counter : 0];
            counter = 0;
        }
    } else {
        ++counter;
    }
}

Did you spot the bug in this loop?  Probably not, because the bug is missing code immediately after the loop.  If the input stream ends during a word, we fail to count it.  We need to repeat the if (counter > 0) test just after the loop, to catch that case.

We have an unusual definition of "word" - in our source code, strings such as ++histo[counter-1]; count as single words.  Even in English text, we'd expect text, to register as a four-character word.  To fix this, we could consider only alphanumerics as word characters, and that could be a good introduction to the functions in <ctype.h>.
while ((c = getchar()) != EOF) {
    if (isalnum(c)) {
        ++counter;
    } else {
        if (counter > 0) {
            ++histo[counter <= MAXLEN ? counter : 0];
            counter = 0;
        }
    }
}

if (counter > 0) {
    ++histo[counter <= MAXLEN ? counter : 0];
}

Enhancement - we could avoid bars going off the right edge of the display, by calculating a scale factor first.  Here's my suggestion:
#define MAXWIDTH 72             /* screen width available for bars */

    double scale = 1.0;
    for (int i = 0;  i <= MAXLEN;  ++i) {
        if (histo[i] * scale > MAXWIDTH) {
            scale = 1.0 * MAXWIDTH / histo[i];
        }
    }

        for (int j = 0;  j < histo[i] * scale;  ++j) { putchar('*'); }

Finally: it's not part of your C code, but there's no need for cat in the test.  We can simply redirect input like this:
./ch1-ex-1-13-01 <ch1-ex-1-13-01.c

Modified code
Applying all my suggestions, I got:
/* Exercise 1-13. Write a program to print a histogram of the lengths of
   words in its input. It is easy to draw the histogram with the bars
   horizontal; a vertical orientation is more challenging. */

/* Solution 1: Horizontal Bars
 * */
#include <ctype.h>
#include <stdio.h>

#define MAXLEN  10  /* Any word longer than this will get counted in the
                       >MAXLEN histogram. */

#define MAXWIDTH 72             /* screen width available for bars */

int main()
{
    int histo[MAXLEN+1] = { 0 }; /* element 0 counts over-length words */

    /* Perform the counting */
    int counter = 0;
    int c;
    while ((c = getchar()) != EOF) {
        if (isalnum(c)) {
            ++counter;
        } else {
            if (counter > 0) {
                ++histo[counter <= MAXLEN ? counter : 0];
                counter = 0;
            }
        }
    }

    if (counter > 0) {
        ++histo[counter <= MAXLEN ? counter : 0];
    }

    /* Calculate sensible scale */
    double scale = 1.0;
    for (int i = 0;  i <= MAXLEN;  ++i) {
        if (histo[i] * scale > MAXWIDTH) {
            scale = 1.0 * MAXWIDTH / histo[i];
        }
    }

    /* how wide are the labels? */
    int width = 1;
    for (int i = MAXLEN;  i >= 10;  i /= 10) {
        ++width;
    }

    /* Write the output */
    for (int i = 1;  i <= MAXLEN;  ++i) {
        printf(" %*d | ", width, i);
        for (int j = 0;  j < histo[i] * scale;  ++j) { putchar('*'); }
        putchar('\n');
    }

    /* over-long words */
    printf(">%*d | ", width, MAXLEN);
    for (int j = 0; j < histo[0] * scale; ++j) { putchar('*'); }
    putchar('\n');
}


Is there a redundancy between the two functions of adding a start node and an end node to my graph? I feel like there is a redundency between:
def _determine_nodes_with_no_predecessors(nodes:List[Node])->List[str]:
    """
    Given the list of nodes returns the identifiers of nodes that doesn't have any predecessor.
    """
    nodes_ids_without_prdecessors = []
    for node in nodes:
        if not node.predecessors:
            nodes_ids_without_prdecessors.append(node.id_)
    return nodes_ids_without_prdecessors

def _add_start_node(graph:Graph):

    start_node = Node.create_start_node(graph)

    graph.add_node(start_node)

    nodes_ids_without_prdecessors = _determine_nodes_with_no_predecessors(graph.nodes)

    # Its successors are the nodes without predecessors in this graph
    for node_id in nodes_ids_without_prdecessors:
        start_node.add_successor(graph.nodes[node_id])

    # Conversly, We add the `start_node` as a predecessor to the nodes without predecessors in this graph
    for node_id in nodes_ids_without_prdecessors:
        node = graph.nodes[node_id]
        node.add_predecessor(start_node)

And this one:
def _determine_nodes_with_no_successors(nodes:List[Node])->List[str]:
    """
    Given the list of nodes returns the identifiers of nodes that doesn't have any successor.
    """
    nodes_ids_without_successors = []
    for node in nodes:
        if not node.successors:
            nodes_ids_without_successors.append(node.id_)
    return nodes_ids_without_successors

def _add_end_node(graph:Graph):

    end_node = Node.create_end_node() #constructor of node

    graph.add_node(end_node)

    end_node = graph.nodes[end_node.id_]

    nodes_ids_without_successors = _determine_nodes_with_no_successors(graph.nodes)

    # Its predecessors are the nodes without successors in this graph
    for node_id in nodes_ids_without_successors:
        end_node.add_predecessor(graph.nodes[node_id])

    # Conversly, We add the `end_node` as a predecessor to the nodes without predecessors in this graph
    for node_id in nodes_ids_without_successors:
        node = graph.nodes[node_id]
        node.add_successor(end_node)

Note: Node is a class that have the attributes predecessors, successors and id_ and the class methods create_dummy_start_node, create_dummy_end_node, add_predecessor and add_successor. Graph is a class that have the attribute nodes and the method add node.
How to write less code in this case? Is it possible to write only two functions that do the whole stuff (instead of four functions)?

You can use the built-in getattr function to extract an attribute by name. Likewise the setattr function to set attributes.
This means you can pass the name of the attribute to a private function, unifying the code:
def _find_nodes_with_no(pred_succ: str, nodes: List[Node]) -> List[Node]:
    """ Find nodes with either no predecessors, or no successors. """

    if pred_succ not in ("predecessors", "successors"):
        raise ValueError("pred_succ must be either 'predecessors' or 'successors'")

    return [node for node in nodes if not getattr(node, pred_succ)]

Your add_start and add_end functions can thus be written to pass the appropriate names:
def add_start(graph: Graph):
    info = ("create_start_node", "predecessors", "successors")
    _add_outer_node(graph, info)

def add_end(graph: Graph):
    info = ("create_end_node", "successors", "predecessors")
    _add_outer_node(graph, info)

Then in _add_outer_node you can do things like:
cmeth, main_links, back_links = info
create_method = getattr(Node, cmeth)
new_node = create_method(graph)

# ...

outer_nodes = _find_nodes_with_no(main_links, graph)

One thing I do question it your "manual" connection of both the predecessor and successor links. It seems like you want those to be reflexive, so why not create an add_predecessor method that automatically creates the reciprocal link? Likewise with add_successor?


Calculating diagonal difference I was trying to solve a problem concerning the difference of the diagonal sums of a matrix (using C99):

#include<stdio.h>

int main(){
    /*n: is the size of the matrix (squared matrix) and it is Entered by the user*/
    int n,line,row,sum_one=0,sum_two=0,dif;
    scanf("%i",&n);
    int array[n+1][n+1];

    for(line=1;line<=n;line++){
        for(row=1;row<=n;row++){
             scanf("%d",&array[line][row]);
        }
     }

     row = n;
     for(line=n;line>=1;line--){
            sum_one=sum_one+array[line][row];
            row--;
     }
     row = n;
     for(line=1;line<=n;line++){
            sum_two=sum_two+array[line][row];
            row--;
     }
     dif = sum_one-sum_two;
     if(dif<0){
         dif = -dif;
     }
     printf("%d \n",dif);
     return 0;
}

But I get in some testcases "terminated due to time out". Is there an efficient way than this algorithm?

There are a number of things that you can do to improve your code.

Understand C arrays

In C, arrays are zero based, so the first element of array is array[0] not array[1].  A number of other things follow from that.

Use whitespace to improve readability

Lines like this:

for(line=1;line<=n;line++){

become much easier to read with a little bit of whitespace:

for (line = 1; line <= n; line++) {

Choose better variable names

Names like line and row are descriptive, but the name array is not so good.  Consider matrix or some other better name.

Use idiomatic C

The code currently includes this line:

for(line=1;line<=n;line++){

But since arrays are zero-based, as mentioned above, the C idiom would be this:

for (line = 0; line < n; line++) {

Similarly, instead of this:

sum_two=sum_two+array[line][row];

Write this:

sum_two += array[line][row];

Consider eliminating variables

The variable dif is only used at the very end of the program. Since it's really the calculation of this number that is the point of the program, it you don't really need to separately keep sum_one and sum_two.  For example the second calculating for loop could be written like this:

for (line = 0; line < n; line++) {
    dif -= array[line][row];
    row--;
}

Check return values for errors

The call to scanf can fail.  You must check the return values to make sure they haven't or your program may crash (or worse) when given malformed input.  Rigorous error handling is the difference between mostly working and bug-free software.  You should strive for the latter.

Decompose your program into functions

All of the logic here is in main in one rather long and dense chunk of code.  It would be better to decompose this into separate functions.

Avoid scanf if you can

There are so many well known problems with scanf that you're usually better off avoiding it.  A better way to handle this would probably be to input a char array (a string in C) and then parse that.  This would allow the code to handle the case that the number input by the user for the array size is not a number.  The current code loops infinitely when that happens.

Consider separating input and output

Right now, the main function reads in the the matrix, and then performs the calculation and then prints the result of the matrix calculation. A more modular (and likely more maintainable) approach would separate these into separate functions.

Consider more memory efficient calculation

In the current code, the entire matrix is stored in memory even though most elements are never used.  Consider instead that the calculation could be done as each line is read, and then the input line discarded.  For very large matrices this could make a significant difference in storage space required by this program.

Eliminate return 0 at the end of main

Since C99, the compiler automatically generates the code corresponding to return 0 at the end of main so there is no need to explicitly write it.


Writing a class to hold Arduino Properties for an Arduino I2C Debugger I have this class which mostly holds a bunch of properties and their setters. I also have wrappers for some functions of another module. Here is the class:

class ARCArduino(object):
    """
    Class to manage Arduino functionality
    """

    def __init__(self, tree_item, port, identifier):
        self.__i2c = ArduinoI2C()
        self.__i2c.setPort(port)
        self.__i2c.connectPort()
        self.__id = None
        self.__i2c_version = None
        self.__switch_version = None
        self.__tree_item = tree_item
        self.__address_list = []
        self.__port = port
        self.__identifier = identifier

    @property
    def id(self):
        if self.__id is None:
            try:
                self.__id = self.__i2c.getID()
            except IOError:
                self.__id = "NO CONNECTION"
        return self.__id

    @property
    def i2c_version(self):
        if self.__i2c_version is None:
            try:
                self.__i2c_version = self.__i2c.i2cVersion()
            except IOError:
                self.__i2c_version = "NO CONNECTION"
        return self.__i2c_version

    @property
    def switch_version(self):
        if self.__switch_version is None:
            try:
                self.__switch_version = self.__i2c.switchVersion()
            except IOError:
                self.__switch_version = "NO CONNECTION"
        return self.__switch_version

    @property
    def tree_item(self):
        return self.__tree_item

    @tree_item.setter
    def tree_item(self, item):
        if type(item) is not wx.TreeItemId:
            return
        self.__tree_item = item

    @property
    def address_list(self):
        return self.__address_list

    @address_list.setter
    def address_list(self, addr_list):
        if type(addr_list) is not list:
            return
        self.__address_list = addr_list

    @property
    def port(self):
        return self.__port

    @port.setter
    def port(self, port):
        if port[:3] != "COM":
            return
        self.__port = port

    @property
    def identifier(self):
        return self.__identifier

    @identifier.setter
    def identifier(self, letter):
        if type(letter) is not str:
            return
        if letter[-1] != ':':
            letter += ':'
        self.__identifier = letter

    def serialCommand(self, command):
        """
        Wrapper for ArduinoI2C serialCommand function
        """
        return self.__i2c.serialCommand(command)

    def close(self):
        self.__i2c.close()

    def connectPort(self):
        self.__i2c.connectPort()

This implementation feels a little dumb to me. Is there a more elegant way to implement something like this?

Here is some further clarification on the functionality of the class:

self.__i2c is an instance of an Arduino I2C communication module that was internally developed at my company. It allows users to send commands to I2C devices over the Arduino's I2C bus. setPort and connectPort are setting and connect a COM port for the specific Arduino.

self.__id is a serial number generated for the Arduino from a company-MySQL database. It is set elsewhere in the program.

self.__switch_version and self.__i2c_version are two floats stored on the Arduino that are the version number of some software running on the Arduino.

self.__tree_item is a wx.TreeItemId (I believe it's stored as just an int) that is the id of the wx.TreeItem stored in a wx.TreeCtrl elsewhere in the program.

self.__address_list is a list of ints which are the I2C addresses of devices discovered on the Arduino's I2C bus.

self.__port is a string which holds the COM port the Arduino is inserted in.

self.__identifier is a letter which identifies the Arduino elsewhere in the program. The program expects the user has multiple Arduinos concurrently being used, so a letter is a quick shorthand to easily identify them.

Here is some feedback I have. I haven't had a chance to work with Arduino much, but there are some things that stand out to me:

 # Nothing to glaring in your constructor, but some tidbits:
 # I might add some defaults.
 # I also noticed you have a lot of items in your constructor.
 # Maybe **kwargs is warranted.

def __init__(self, tree_item, port=8080, identifier):

or

def __init__(self, **kwargs)

    # the number of arguments is borderline imho. I usually find if I
    # have too large a number (subjective), there is opportunity to
    # break out the code into more classes.
    self.__i2c = ArduinoI2C()
    self.__i2c.setPort(port)
    self.__i2c.connectPort()
    self.__id = None
    self.__i2c_version = None
    self.__switch_version = None
    self.__tree_item = tree_item
    self.__address_list = []
    self.__port = port
    self.__identifier = identifier

# Typically methods indicate some sort of action. i.e. get_id, show_id.
# specifying a noun is usually grounds for a class/instance variable.
# I would consider renaming your method to "def get_id(self):"
@property
def id(self):
    if self.__id is None:
        try:
            self.__id = self.__i2c.getID()
        except IOError:
        # It may be good to add some logging.
        # python logging module is great.
            self.__id = "NO CONNECTION"
    return self.__id

# ditto method name
@property
def i2c_version(self):
    if self.__i2c_version is None:
        try:
            self.__i2c_version = self.__i2c.i2cVersion()
        except IOError:
        # ditton on the logging.
            self.__i2c_version = "NO CONNECTION"
    return self.__i2c_version

# ditto method name
@property
def switch_version(self):
    if self.__switch_version is None:
        try:
            self.__switch_version = self.__i2c.switchVersion()
        except IOError:
            self.__switch_version = "NO CONNECTION"
    return self.__switch_version

# ditto method name
@property
def tree_item(self):
    return self.__tree_item

# ditto method name
@tree_item.setter
def tree_item(self, item):
    if type(item) is not wx.TreeItemId:
        return
    self.__tree_item = item

# ditto method name
@property
def address_list(self):
    return self.__address_list

# ditto method name
@address_list.setter
def address_list(self, addr_list):
    if type(addr_list) is not list:
        return
    self.__address_list = addr_list

# ditto method name
@property
def port(self):
    return self.__port

# ditto method name
@port.setter
def port(self, port):
    if port[:3] != "COM":
        return
    self.__port = port

# ditto method name
@property
def identifier(self):
    return self.__identifier

# ditto method name
@identifier.setter
def identifier(self, letter):
    if type(letter) is not str:
        return
    if letter[-1] != ':':
        letter += ':'
    self.__identifier = letter

# ditto, and its not idiomatic to use camelCase in a method name.
#   def run_serial_command(self, command)
def serialCommand(self, command):
    """
    Wrapper for ArduinoI2C serialCommand function
    """
    return self.__i2c.serialCommand(command)

# this method name is ok
def close(self):
    self.__i2c.close()

# this one two.. ditch the camelCase for snake_case connect_port
def connectPort(self):
    self.__i2c.connectPort()

It looks like there might be some opportunity for grouping your commands into separate classes, however not sure if you have enough per group yet.

i.e.

class ArduinoVersions(ARCArduino):

# this might be to ambiguous, but just trying to give the general idea.
class ArduinoProperties(ArcaArduino):

I have a feeling your command set will grow, so breaking this out may provide better organization in the future.


Refactor ruby each-code def users_list
  html = ''
  self.users.each do |user|
    html << user.link_avatar_tag
  end
  html.html_safe
end

I feel that it is possible to write shorter.

Is it possible get rid of the temporary variable(html)?

You should use map (don't use each to accumulate. Check this page about functional programming):

def users_list
  users.map(&:link_avatar_get).join.html_safe
end