Yellow Pages scraper What are your thoughts and comments relating to areas I could improve on?

import csv
import re
import requests
from bs4 import BeautifulSoup

class Resto:
    def __init__(self,url,filename="default"):
        self.page = 1
        url = url + "&page={0}"
        self.url = url
        self.filename = filename

    def info(self):
        linkz = []
        while True:
            content = requests.get(self.url.format(self.page)).content
            soup = BeautifulSoup(content)
            links = [i["href"] for i in soup.find_all("a",{"itemprop":"name"})]
            if not links:
                break
            linkz.extend(links)
            self.page += 1
        return linkz

    def email(self):
        names = []
        emails = []
        sites = []
        streets = []
        cities = []
        numbers = []
        for i in self.info():
            i = "http://www.yellowpages.com" + i
            content2 = requests.get(i).content
            soup2 = BeautifulSoup(content2)
            name = [i.text for i in soup2.find_all("h1",{"itemprop":"name"})]
            email = [re.sub(r'mailto:', "",i["href"]) for i in soup2.find_all("a",{"class":"email-business"})]
            if email:
                email = [email[0]]
            site = [i["href"] for i in soup2.find_all("a",{"class":"custom-link"}) if i.text == "Visit Website"]
            street = [i.text for i in soup2.find_all("p",{"class":"street-address"})]
            city = [i.text for i in soup2.find_all("p",{"class":"city-state"})]
            phone = [i.text for i in soup2.find_all("p",{"class":"phone"})]

            if not email:
                email = ["---"]
            if not site:
                site = ["---"]

            if not street:
                street = ["---"]
            if not city:
                city = ["---"]
            if not phone:
                phone = ["---"]

            names.extend(name)
            emails.extend(email)
            sites.extend(site)
            streets.extend(street)
            cities.extend(city)
            numbers.extend(phone)
        return names,streets,cities,numbers,sites,emails

    def csv(self):
        """saves csv file in root of python 3 directory"""
        items = self.email()
        items = list(zip(*items))
        with open("{0}.csv".format(self.filename), "w")as shops:
            writer = csv.writer(shops)
            writer.writerows(items)

a = Resto("http://www.yellowpages.com/search?search_terms=restaurants&geo_location_terms=tx")
a.csv()

You have an awful lot of excess whitespace. Use one or two blank lines between functions, and one blank line to break up different parts of each function (especially, just below the end of a loop or if block). Don't put blank lines immediately between a line ending in : and the start of the indented block beneath it.

The while loop in info uses a class attribute as a loop counter, which actually seems to cause a potential bug here: the counter persists, so on subsequent calls you will skip the loop, never append to the outer list, and it will be returned empty. If page is a local variable instead, and it will work fine no matter how many calls you make.

Instead of keeping a loop counter like this, and incrementing it manually, preferred Python style is to use a for loop. The equivalent for loop to your current while loop would be:

for page in range(1, 2):
   ...

Although it would be good to store the maximum page in a variable. But in light of your clarification in the comments that this would normally be a while True: loop, which only breaks once it hits a page with no interesting links, then the equivalent is to use itertools from the stdlib:

for page in it.count(1):
    ...

You have a variable links to capture the links on each iteration of the loop, and linkz to accumulate them across all the pages you parse (which is only one at the moment anyway, but I'm assuming that could change later). Those names are quite confusing; it would be good to differentiate better - call them links for the one that gathers all the links, and page_links for the one that gets the links for just this page.

Your email function parses what I would call your 'main' data structure. Since that is a lot more than just email-related information, email isn't the best name for it.

The data structure that email creates seems awkward for your data. If you think of people as 'records', and the information you capture about them to be 'fields', then it would make more sense to have it as a list of namedtuples, or or even a pandas dataframe if you're working with a lot of data. That way, your code to parse it is a little bit simpler:

person = namedtuple(...)
people = []
for link in links:
   this_person = person()
   person.phone = #parse phone number
   etc.

return people

But this is the major data structure in your class. It contains all the information that this class is designed to parse. So, I think all of this should be done in __init__, with this structure - and only this structure - captured as an attribute.

The only other attribute you store is the filename that gets passed to __init__. I think it would make more sense as an argument to your csv method - the only method that uses it - because the filename to write it to is a decision that goes with "I want to write this data to disk" more than "I want to scrape some data from a website".

So, you're left with a class with one attribute, and two methods - one of which is __init__, and the other is fairly trivial. In fact, csv becomes even more trivial with a list of namedtuples, since you wouldn't need to zip it. Similarly if you use a pandas DataFrame, it has it's own CSV routines that you can use.

That means there is no need for a class. Instead, use what your __init__ has grown to as a standalone function - call it parse_yellowpages(url). Have it return the list or DataFrame. Then drop the csv function entirely and just put that in the main line of your program.

Your current code is a little schizophrenic about whether it allows multi-valued fields: eg, can a person have more than one phone number?

The code parsing it looks like it does. You parse out a list of data, then you extend it onto the accumulated list. But then when you write out your CSV, you assume (by your use of zip to regroup them) that all fields have exactly one value.

In the comments you clarified that you only want single-valued fields. That being the case, don't include a full list of every match you found in your data structure. If you get three phone numbers for a person when you're only expecting one, you want to either consider it an error and bail out, or forget all but one of them. Your current code would silently remember all of them, forget that it did that, and end up with corrupt data.

To take the 'signal an error' path, do something like this:

phone_numbers = soup2.findall(...)
if len(phone_numbers) != 1:
    raise RuntimeError('Empty or multivalued field: ' + url)
else:
   this_person.phone = phone_numbers[0]

If you prefer to instead ignore any surplus values, use find instead of find_all - they take the same arguments, but find will just return one result instead of a list of them.


Cryptopals first challenge - hexadecimal to base64 I've just started the cryptopals-challenge, and now wanted to show my solution to the first challenge here:

public class Challenge1_1 {

    public static void main(String[] args) {
        String hex = "49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d";
        String base64 = hextobase64(hex);
        System.out.println(base64);
        String test = "SSdtIGtpbGxpbmcgeW91ciBicmFpbiBsaWtlIGEgcG9pc29ub3VzIG11c2hyb29t";
        boolean t = base64.equals(test);
        System.out.println(t);
    }

    public static String hextobase64(String hex) {

        String[] hex_array = new String[hex.length()/2];

        //Splits up the hex String into substring with length 2
        for(int i = 0; i < hex_array.length; i++) {
            int x = 2 * i;
            int y = x + 2;
            hex_array[i] = substring(hex, x, y);
        }

        //Conversion to binary system
        String binary = "";
        String bin = "";
        for(int i = 0; i < hex_array.length; i++) {
            bin = conversion(hex_array[i]);
            while(bin.length()<8){
                bin = "0" + bin;
            }
            binary = binary + bin;
        }

        //Split up to strings of length 6
        String[] binary6 = new String[binary.length()/6];
        for(int i = 0; i < binary6.length; i++) {
            int x = i * 6;
            int y = x + 6;
            binary6[i] = substring(binary, x, y);
        }

        //Conversion to base64
        String out = "";
        String character = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
        String[] base64 = {
                "000000", "000001", "000010", "000011", "000100", "000101", "000110", "000111", "001000", "001001", "001010", "001011", "001100", "001101", "001110", "001111",
                "010000", "010001", "010010", "010011", "010100", "010101", "010110", "010111", "011000", "011001", "011010", "011011", "011100", "011101", "011110", "011111",
                "100000", "100001", "100010", "100011", "100100", "100101", "100110", "100111", "101000", "101001", "101010", "101011", "101100", "101101", "101110", "101111",
                "110000", "110001", "110010", "110011", "110100", "110101", "110110", "110111", "111000", "111001", "111010", "111011", "111100", "111101", "111110", "111111"
        };
        for(int i = 0; i < binary6.length; i++) {
            for(int j = 0; j < base64.length; j++) {
                if(binary6[i].equals(base64[j])){
                    out = out + character.charAt(j);
                }
            }
        }
        return(out);
    }

    public static String conversion(String hex) {
        //hex-system to decimal system
        String hex_numbers = "0123456789ABCDEF";
        hex = hex.toUpperCase();
        int value = 0;
        for (int i = 0; i < hex.length(); i++)
        {
            int d = hex_numbers.indexOf(hex.charAt(i));
            value = 16*value + d;
        }

        //decimal to binary
        String out = "";
        while(value != 0){
            int mod = value%2;
            String m = mod + "";
            value = value/2;
            out = m + out;

        }
        return out;

    }

    public static String substring(String str, int start, int end) {

        String out = "";
        if (start > end) {
            return out;
        }

        if (start < 0) {
            start = 0;
        }

        if (end > str.length() - 1) {
            end = str.length();
        }

        while (start < end) {
            out = out + str.charAt(start);
            start = start + 1;
        }

        return out;

    }
}

I've tried to all do it manually instead of using some kind of java-package.
I would appreciate any suggestions to improve the code.

That is a very pure solution that does not use any available feature. It is a solid solution.

However everything is String, even the conversion from a byte as two hexadecimal digits uses integer, but converts it back to a string.

The same code style of yours would allow immediately convert every hexadecimal digit to 4 bits.

    final String[] nibbles = { "0000", "0001", "0010", "0011",
            "0100", "0101", "0110", "0111",
            "1000", "1001", "1010", "1011",
            "1100", "1101", "1110", "1111" };
    int nbitsRaw = hex.length() * 4;

    // Make nbits a multiple of 6
    int nbits += (6 - (nbitsRaw % 6)) % 6;
    StringBuilder sb = new StringBuilder(nbits);
    hex.codePoints()
            .forEach(hexdigit -> {
                int value = hexdigit <= '9' ? hexdigit - '0' : 9 + (hexdigit & 0xF); // 0-9A-Fa-F
                sb.append(nibbles[value]);
            });
    for (int i = nbitsRaw; i < nbits; ++i) {
        sb.append('0');
    }

Best of course would be using the bits in an int, not needing to juggle string constants of binary numbering. Especially the loops hurt. One would indeed far better use a Map<String, Character> but I understand your requirement of not using any higher construct.

The code is necessarily slow. You can try a longer input, and will probably have to wait for a result.

String concatenation is slow; char[] or StringBuilder would be advisable.
One can always do new String(charArray).

Stylistic:

hextobase64 by java camel case convention: hexToBase64
conversion no-namer: hexByteToBits
The constants could be fields private static final String[] BASE64 i.o. base64.


Spring AOP Log4j Logger example, improvement I went through some of the stuffs on net regarding spring AOP and wrote a Spring AOP logger.

Can you guys check my code and give your valuable inputs to improve it or if it is not proper, give me an another example of how can I achieve it?

My Code is as below :

@Aspect
public class SpringAOPLogger {

    @Around("execution(* com.sample.dao.*.*(..)) || execution(* com.sample.service.*.*(..))"
            + "|| execution(* com.sample.controller.*.*(..))")
    public Object logMethod(ProceedingJoinPoint joinPoint) throws Throwable {
        String className = joinPoint.getTarget().getClass().getSimpleName();
        final Logger logger = Logger.getLogger(joinPoint.getTarget().getClass());
        Object retVal = null;

        try {
            StringBuffer startMessageStringBuffer = new StringBuffer();

            startMessageStringBuffer.append("Entered in ").append(className).append(".");
            startMessageStringBuffer.append(joinPoint.getSignature().getName());
            startMessageStringBuffer.append("(");

            Object[] args = joinPoint.getArgs();
            for (Object arg : args) {
                startMessageStringBuffer.append(arg).append(", ");
            }
            if (args.length > 0) {
                startMessageStringBuffer.deleteCharAt(startMessageStringBuffer.length() - 1);
            }

            startMessageStringBuffer.append(")");

            //Method Entry Log
            logger.debug(startMessageStringBuffer.toString());

            StopWatch stopWatch = new StopWatch();
            stopWatch.start();

            retVal = joinPoint.proceed();

            stopWatch.stop();

            StringBuffer endMessageStringBuffer = new StringBuffer();
            endMessageStringBuffer.append("Exit from ").append(className).append(".");
            endMessageStringBuffer.append(joinPoint.getSignature().getName());
            endMessageStringBuffer.append("(..); Execution time: ");
            endMessageStringBuffer.append(stopWatch.getTotalTimeMillis());
            endMessageStringBuffer.append(" ms;");

            //Method Exit Log
            logger.debug(endMessageStringBuffer.toString());
        } catch (Throwable ex) {
            StringBuffer errorMessageStringBuffer = new StringBuffer();
            // Create error message
            logger.error(errorMessageStringBuffer.toString(), ex);
            throw ex;
        }
        return retVal;
    }
}

and below is the spring xml configuration for the same :

    <aop:aspectj-autoproxy>
        <aop:include name="springAopLogger" />
    </aop:aspectj-autoproxy>

    <bean id="springAopLogger" class="com.sample.utility.SpringAOPLogger" />

Use StringBuilder instead of StringBuffer since the latter is synchronized and you're not sharing the instances in a multi-threading manner. So use of StringBuilder would be a bit faster.
Narrow down the scope of retVal. Generally speaking, your variables should be as close to the use places as possible. Note that you can move the retVal declaration and the return statement to the try block entirely.
A matter of style, but I would use final as much as possible.
Concatenate the @Around expression into a single constant if possible and if it's not disallowed by your code convention.
Split your single method into smaller ones. First off, you separate different responsibilities and improve the readability. Note that the message formatting methods return String (why should the call site know what's behind the scenes?). You can shorten the behind-the-scenes object names to simple builder as there are no name conflicts. These methods are declared static, and they use JoinPoint instead of ProceedingJoinPoint just because it's enough to accept an instance of the first class. And getPostMessage is better to accept long and not StopWatch -- what if you would use System.currentTimeMillis()?
StringBuilder is a perfect example of the fluent interface concept, so you might want even use it more (and even discard its variables: see getPostMessage with a single return but multiple chained appends).
Joining a sequence of items using a separator is a trivial problem, and you could eliminate use of deleteCharAt. Just ignore the very first "imaginary" separator and not append it. Or use Guava Joiner.appendTo(StringBuilder,Object[]) if possible (can't recall if there is a Java 8 StringBuilder-friendly collector). Note that appendTo() from the answer is really an utility-candidate method so you might want to reuse it elsewhere if it's justified.
targetClass is introduced, because I don't know if getting the target is cheap (I believe it's extremely cheap, but this is a particular case -- but this is an interface, and what if you mock your JoinPoint to unit-test the logger and don't want the getTarget code behind to happen twice?). However, having a variable named targetClass is a good self-describing hint.
Also, usually, a try/catch block can cover the whole method, thus className can be moved into it safely without changing the semantics (but not logger because it's used in separate blocks).
Static imports may improve readability, so maybe just getLogger(...) instead of Logger.getLogger(...)?
And one more thing that I'm not sure about, and it would require more investigation: StringBuilder can be replaced with string concatenation. At least for simple cases like https://gist.github.com/lyubomyr-shaydariv/ffdc40df722ca489e14364da8a8c4e30 (those two methods produce the same Java byte-code). See more at https://stackoverflow.com/questions/1532461/stringbuilder-vs-string-concatenation-in-tostring-in-java .

@Aspect
public class SpringAOPLogger {

    @Around("execution(* com.sample.dao.*.*(..)) || execution(* com.sample.service.*.*(..)) || execution(* com.sample.controller.*.*(..))")
    public Object logMethod(final ProceedingJoinPoint joinPoint)
            throws Throwable {
        final Class<?> targetClass = joinPoint.getTarget().getClass();
        final Logger logger = getLogger(targetClass);
        try {
            final String className = targetClass.getSimpleName();
            logger.debug(getPreMessage(joinPoint, className));
            final StopWatch stopWatch = new StopWatch();
            stopWatch.start();
            final Object retVal = joinPoint.proceed();
            stopWatch.stop();
            logger.debug(getPostMessage(joinPoint, className, stopWatch.getTotalTimeMillis()));
            return retVal;
        } catch ( final Throwable ex ) {
            logger.error(getErrorMessage(ex), ex);
            throw ex;
        }
    }

    private static String getPreMessage(final JoinPoint joinPoint, final String className) {
        final StringBuilder builder = new StringBuilder()
                .append("Entered in ").append(className).append(".")
                .append(joinPoint.getSignature().getName())
                .append("(");
        appendTo(builder, joinPoint);
        return builder
                .append(")")
                .toString();
    }

    private static String getPostMessage(final JoinPoint joinPoint, final String className, final long millis) {
        return new StringBuilder()
                .append("Exit from ").append(className).append(".")
                .append(joinPoint.getSignature().getName())
                .append("(..); Execution time: ")
                .append(millis)
                .append(" ms;")
                .toString();
    }

    private static String getErrorMessage(final Throwable ex) {
        return ex.getMessage();
    }

    private static void appendTo(final StringBuilder builder, final JoinPoint joinPoint) {
        final Object[] args = joinPoint.getArgs();
        for ( int i = 0; i < args.length; i++ ) {
            if ( i != 0 ) {
                builder.append(", ");
            }
            builder.append(args[i]);
        }
    }

}


Build all possible phrase pairs from a sentence pair I have an array for an English sentence (enSentence) and for a German sentence (deSentence). Each value represents one word. I want to print every possible phrase pair (enPhrase and dePhrase), where no phrase contains more than maxhPhraseLength words. A phrase can consist of a single word, all words of the sentence, and everything inbetween (consecutive).

So for Hello world ! and Hallo Welt !, possible phrase pairs would be:

Hello - Hallo
Hello world - Hallo
Hello world ! - Hallo
world - Hallo
world ! - Hallo
! - Hallo
…
Hello - Hallo Welt
Hello - Hallo Welt !
Hello - Welt
…
Hello world ! - Hallo Welt !

But phrases like Hello ! or world Hello are not possible.

This is my first try:

enLoop:
for(int en1 = 0; en1 < enSentence.length; en1++) {
  for(int en2 = en1; en2 < enSentence.length; en2++) {

    if(en2-en1+1 > maxPhraseLength) {
      continue enLoop;
    }

    deLoop:
    for(int de1 = 0; de1 < deSentence.length; de1++) {
       for(int de2 = de1; de2 < deSentence.length; de2++) {

         if(de2-de1+1 > maxPhraseLength) {
           continue deLoop;
         }

         // building the phrase pair
         enPhrase = "";
         dePhrase = "";
         for(int e = en1; e <= en2; e++) {
           enPhrase = enPhrase + " " + enSentence[e];
         }
         for(int d = de1; d <= de2; d++) {
           dePhrase = dePhrase + " " + deSentence[d];
         }
         enPhrase = enPhrase.trim();
         dePhrase = dePhrase.trim();
         System.out.println(enPhrase + " - " + dePhrase);

       }
     }

  }
}

I’m interested in any kind of feedback (I rarely create code from scratch).

I considered naming the variables language-neutral (in case I want to use the code for making phrase pairs in other languages, too), but couldn’t come up with something short and catchy (sourceSentence/source1 and targetSentence/target1 are longer, and I always have to remind myself what the source and what the target is, while it’s immediately clear to me if thinking about the terms en/English and de/German instead; but the difference doesn’t really matter in this case anyway).

Repeating work

You should move all of the code that computes enPhrase to before the deLoop.  There is no need to recompute enPhrase each time you modify de2.

Furthermore, when you compute dePhrase, you can do so by appending the next word deSentence[de2] to the current dePhrase.  Right now you reconstruct all of dePhrase in the inner loop, making it an \$O(n)\$ operation when it could be an \$O(1)\$ operation.

Get rid of loop labels

You can get rid of the loop labels if you precalculate the proper end of the loop.

Use StringBuilder

It's more efficient to use a StringBuilder to concatenate strings rather than using the + operator.

Rewrite

Here is a rewrite showing the above changes:

StringBuilder enPhrase = new StringBuilder();
StringBuilder dePhrase = new StringBuilder();

for(int en1 = 0; en1 < enSentence.length; en1++) {
  int en2Max = Math.min(enSentence.length, en1+maxPhraseLength);
  for(int en2 = en1; en2 < en2Max; en2++) {
    if (en2 == en1) {
      enPhrase.setLength(0);
    } else {
      enPhrase.append(" ");
    }
    enPhrase.append(enSentence[en2]);
    for(int de1 = 0; de1 < deSentence.length; de1++) {
       int de2Max = Math.min(deSentence.length, de1+maxPhraseLength);
       for(int de2 = de1; de2 < de2Max; de2++) {
         if (de2 == de1) {
           dePhrase.setLength(0);
         } else {
           dePhrase.append(" ");
         }
         dePhrase.append(deSentence[de2]);

         System.out.print(enPhrase.toString());
         System.out.print(" - ");
         System.out.println(dePhrase.toString());
       }
     }

  }
}


Building object identity in PHP I am trying to build object identity in PHP so that when I have a collection of objects, each one can have a string as identifier and all of these identifiers are afterwards combined to form an unique md5 to represent the "identity of a collection".

Why? So that I can choose to skip re-execution of code when it's not needed:

interface SomeTestInterface
{
    public function testFunction();
}

abstract class Identifiable
{
    public function __toString()
    {
        $identity_shards = array_merge( get_object_vars( $this ), class_implements( $this ) );

        $identity_string = '';

        foreach( $identity_shards as $identity_shard_key => $identity_shard_value ) {
            $identity_string .= (string) $identity_shard_key . (string) json_encode( $identity_shard_value );
        }

        return md5( get_class( $this ) . $identity_string );
    }
}

class SomeBaseClass extends Identifiable implements SomeTestInterface
{
    public function __construct( $number )
    {
        $this->number = $number;
        $this->thing = 'a';
        $this->other_thing = ['a','b','c',1,2,3,];
    }

    public function testFunction()
    {
        return 'a';
    }
}

This is testable with:

for( $i = 1; $i < 10000; $i++ ) {
    $class = new SomeBaseClass( $i );
    (string) $class;
}

For me, PHP 7.3 and WordPress, this takes ~100ms to execute.

My micro-decisions:

I need json_encode on $identity_shard_value because you can't cast an array to string, for example. json_encode is both fast in my experience and knows how to deal with it all.
I chose to make this an abstract class because json_encode doesn't have access to scoped classes, as such, it cannot encode what it can't find, so I must be able to access $this though it's weird because even in the abstract class, I still can't encode it, but I should be able to.

My main concerns with this is if I really need all these items to build my object identity or if there's another, faster way. 10000 objects in 0.1ms, although very good on its own, doesn't necessarily scale.

In essence, every single object that implements Identifiable in a collection that a module of my framework has will have an identity that I will then combine into a final "collection identity" to later do a check such as:

$collection_identity = getCollectionIdentity( $collection ); //MD5 computed from the identity of all these objects

if( $collection_identity != getCollectionIdentityByName( 'some_collection' ) {
    setCollectionIdentity( 'some_collection', $collection_identity );
    //re-execute code
} else {
 retrieveDataFromStorage();
}

As you can see, it checks if there was a change to the objects / collection and if so, it re-executes all the other code, but if not, it just retrieves what that "other code" generated in the past and as such, this is a way to use persistent storage to skip execution of heavy code.

I think this code is fine, and can't be sped up very much. But ....

The MD5 hash is most likely unique, it's got 16^32 (3.4e38) values after all, but once in a blue moon two different objects will have the same identity, especially if you use this a lot. This might cause very rare, random, bugs in your software. Bugs that are virtually impossible to track down.

I don't think the __toString() magic Method was intended for the purpose you're now using it for. I have learned that; "You should always use something for the purpose it was intended for.". The purpose of __toString() is to give you a readable representation of the object. By appropriating it now for identifying objects, you're loosing the capability to use it for its intended purpose later.

You're also relying on an undocumented property of get_object_vars(), namely that it will always return the variables in the same order. Will it? I don't know. It probably will, but doesn't have to. This could also change with changing versions of PHP, leaving you with a very big headache if it happens. You could use ksort() to make sure the order is always the same, but that will slow things down a lot.

I've also read in various places, and in the comments in the manual, that get_object_vars() doesn't return static variables. That makes sense since all objects of a class share the same values for these variables, but it is something to keep in mind.

The storing and checking of the identity hashes, in some collection of hashes, will probably be the slowest part of this whole idea.

Then my final problem wilt this code:

Properly written code would know the identity of its objects, or at least have a 100% reliable method to check this. Your code should be written in such a way that it already minimizes object duplication. This code seems the result of not being able to write good and efficient code (sorry, I'm trying to make a point here).

For instance, many objects could already have a simply ID integer that identifies them. For instance a model class, based on a database row, would most likely have such an ID. Most other classes could, if needed, have a similar way to identify themselves. Once you combine such an ID with the class name you should have a 100% reliable identifier.

If you really need a way to identify various objects you could simply add an identity() method to them. Something like this:

<?php

class MyClass
{
    public function __construct($id)
    {
        $this->id = $id;
    }

    public function identity() {
        return get_class() . ":" . $this->id;
    }
}

$myObject = new MyClass(999);

echo $myObject->identity();

?>

This would return:

  MyClass:999

I agree that this is a very basic example, but it should be possible to do something similar for any class.

By writing such a specific identifier method for each class you can optimize it, which means it will be faster, and you can make it a 100% reliable under any circumstances. It is also a lot easier to debug, because you can see and read what is going on. No hiding behind mysterious hashes here.

Conclusion: Despite my objections I think your code looks fine. I do however wonder whether this approach will, in the end, cause more trouble than it is worth.

Note: There is more discussion in the comments. In the end coolpasta wrote a response to this question.


Building a string with only two repeating characters The below is the C# implementation for the a function for given parameters int a, int b - both represent characters 'A' and 'B' respectively, where the function should return a string containing both characters 'A' and 'B' occurring a times and b times respectively but neither 'A' nor 'B' repeating consecutively for more than 2 times. Both values for a and b are given in a way that a string can be build using those numbers - so eg: Foo(0,3) or Foo(1,7) shall not be invoked.
eg:
Foo(3,3) returns "BBAABA" or "AABBAB"
Foo(4,1) returns "AABAA"
Foo(3,5) returns "BAABBABB" or "BBAABBAB"
Code:
static string Foo(int a, int b)
{
    int total = a + b;
    StringBuilder sb = new StringBuilder();
    char charToPrint = a > b ? 'A' : 'B';
    int flag = 0;

    for(int x =0; x< total; x++)
    {
        if(flag == 2)
        {
            flag = 0;
            charToPrint = SwapChar(charToPrint);
        }
        if(charToPrint == 'A' && a == 0 || charToPrint == 'B' && b == 0)
        {
            charToPrint = SwapChar(charToPrint);
        }

        if (charToPrint == 'A')
            a--;
        else
            b--;

        sb.Append(charToPrint);
        flag++;
    }
    return sb.ToString();
}

static char SwapChar(char thisChar)
{
    return thisChar == 'A' ? 'B' : 'A';
}

This is working but I would like to receive some feedback on this code.

Your algorithm has a flaw - it's too greedy. Take this example:
Foo(6, 2) // "AABBAAAA"

Alas, it should return "AABAABAA".
In terms of a review, your method shouldn't be called Foo - give it a descriptive name.
Great use of StringBuilder. You could initialize the length to a+b in the constructor as you know what size your finished string will be.
Expression bodied members can greatly improve readability in my opinion:
static char SwapChar(char c) =>
    c == 'A' ? 'B' : 'A';

Try to avoid variables like flag. I had to read the whole loop before I knew what that variable meant. I'd call it something like repeatCount.


Extract html content based on tags, specifically headers I want the function to take as an input json file containing html_body with its corresponding url and return list of tuples containing headers and their corresponding url (so could be tuple with one item as a list of the headers).

So far I have been able to do so, but I'm missing some headers and more over, I'm sure that this code can be improved.

Commented out possible extensions for <h2> headers extraction, and a different regex pattern I tried.

A taste of the source:

[["<body class=\" catalog-category-view categorypath-sale-html category-sale\">\n<script type=\"text/javascript\">\n//<![CDATA[\nif (typeof(Varien.searchForm) !== 'undefined') {\n    Varien.searchForm.prototype._selectAutocompleteItem = function(element) {\n        var link = element.down();\n        if (link && link.tagName == 'A') {\n            setLocation(link.href);\n        } else {\n            if (element.title){\n                this.field.value = element.title;\n            }\n            this.form.submit();\n        }\n    };\n    Varien.searchForm.prototype.initAutocomplete = function(url, destinationElement) {\n        new Ajax.Autocompleter(\n            this.field,\n            destinationElement,\n            url,\n            {\n                paramName: this.field.name,\n                method: 'get',\n                minChars: 2,\n                frequency: .3,\n                updateElement: this._selectAutocompleteItem.bind(this),\n                onShow : function(element, update) {\n                    if(!update.style.position || update.style.position=='absolute') {\n                        update.style.position = 'absolute';\n                        Position.clone(element, update, {\n                            setHeight: false,\n                            offsetTop: element.offsetHeight\n                        });\n                    }\n                    Effect.Appear(update,{duration:0});\n                }\n\n            }\n        );\n    };\n    Autocompleter.Base.prototype.markPrevious = function() {\n        if (this.index > 0) {\n            this.index--;\n        } else {\n            this.index = this.entryCount - 1;\n        }\n        var entry = this.getEntry(this.index);\n        if (entry.select('a').length === 0) {\n            this.markPrevious(); // Ignore items that don't have link\n        }\n    };\n    Autocompleter.Base.prototype.markNext = function() {\n        if (this.index < this.entryCount - 1) {\n

Code:

from pandas import DataFrame, Series
from tqdm import tqdm
import html5lib
import re
from bs4 import BeautifulSoup

def get_headers_from_json(local_path):
"""
The function takes a json file with html_body and returns a list of headers.
It parses the headers, based on tags starting with 'h'.
It also parses urls and return them in a tuple: (headers, url)
"""
data = json.loads(open(local_path).read())
pattern = "h1" # | h2 | h3"
# pattern = re.compile("^(?!.*(href)).*^h", re.IGNORECASE)
headers_urls = []
all_headers = []
for x in tqdm(data):
    soup = BeautifulSoup(x[0], 'html.parser')
    html_headers = soup.find_all(re.compile(pattern))
    all_headers.append(html_headers)
    url = x[1]
    tuple_title_url = all_headers, url
    headers_urls.append(tuple_title_url)
return headers_urls

PS - I checked the following links but couldn't find a great solution:
HTML tag encoder
HTML parsing algorithm for extracting <a> tags
Scraping HTML using Beautiful Soup

Currently, you are matching a single h1 tag only, but, from what I understand, you want to catch all the "header" elements. There are multiple ways to do that.

One is with a regular expression:

headers = soup.find_all(re.compile(r"^h"))

Or, with a lambda function:

headers = soup.find_all(lambda tag: tag and tag.name.startswith("h"))

Or, with a list of explicitly specified tags:

headers = soup.find_all(['h1', 'h2', 'h3', 'h4', 'h5'])

Note that in order to get the header texts, you would use .get_text() method:

[header.get_text() for header in headers]

Other notes:

if every item returned from tqdm(data) consists of two items, you can unpack them in a for loop:

for html, url in tqdm(data):

Then, you can use html instead of x[0] and url instead of x[1].
if you are going to go with a regular expression approach to match the headers, you should pre-compile the expression - define it as a constant:

HEADER_PATTERN = re.compile(r"^h")

you can use json.load() to read JSON from a file directly, replace:

data = json.loads(open(local_path).read())

with:

with open(local_path) as input_file:
    data = json.load(input_file)

If performance matters:

you can use a faster JSON parser: ujson instead of json
use lxml instead of html.parser as a BeautifulSoup underlying parser
you can parse header elements only with SoupStrainer ignoring all other elements - this should boost HTML parsing performance as well

Improved code (with some of the suggestions applied):

import re

from bs4 import BeautifulSoup, SoupStrainer
from tqdm import tqdm
import ujson

HEADER_PATTERN = re.compile(r"^h")

def get_headers_from_json(local_path):
    """
    The function takes a json file with html_body and returns a list of headers.
    It parses the headers, based on tags starting with 'h'.
    It also parses urls and return them in a tuple: (headers, url)
    """
    with open(local_path) as input_file:
        data = ujson.load(input_file)

    parse_only = SoupStrainer(HEADER_PATTERN)
    return [
        ([header.get_text() for header in BeautifulSoup(html, 'lxml', parse_only=parse_only)(HEADER_PATTERN)], url)
        for html, url in tqdm(data)
    ]


Add Default Rubberduck VBA Folder Annotation to VBProject.VBComponents Currently Rubberduck VBA files all the VBProject VBComponents in that do not have a Folder Annotation in a Folder named after the VBProject.  It can be time consuming to manually organize the components for the first time and a inconvenient to add annotations to every Worksheet, Userform, Class, ...ect.  When ran my code will organize any unfiled component by ProjectName.ComponentType.
Before

After

You will need to add a reference to the Microsoft Visual Basic for Applications Extensibility 5.3 and from file options check "Trust access to VBA project object model".

Private Sub AddDefaultFolderAnnotationsToVBComponents()
    Dim Component As VBComponent
    Dim Project As VBProject

    Select Case Application.Name
    Case "Microsoft Access", "Microsoft Word"
        Set Project = IIf(True, Application, 0).VBE.ActiveVBProject

    Case "Microsoft Excel"
        Set Project = IIf(True, Application, 0).ActiveWorkbook.VBProject

    Case "Microsoft PowerPoint"
        Set Project = IIf(True, Application, 0).VBE.VBProjects(1)

    End Select

    Dim FolderName As String
    Dim HasFolder As Boolean

    For Each Component In Project.VBComponents
        With Component.CodeModule
            If .CountOfLines = 0 Then
                HasFolder = False
            Else
                HasFolder = InStr(.Lines(1, .CountOfLines), Chr(39) & "@Folder")
            End If
        End With

        If Not HasFolder Then
            Select Case Component.Type
                Case vbext_ComponentType.vbext_ct_StdModule
                    FolderName = Project.Name & ".Modules"
                Case vbext_ComponentType.vbext_ct_ClassModule
                    FolderName = Project.Name & ".Classes"
                Case vbext_ComponentType.vbext_ct_MSForm
                    FolderName = Project.Name & ".Forms"
                Case vbext_ComponentType.vbext_ct_ActiveXDesigner
                    FolderName = Project.Name & ".Designers"
                Case vbext_ComponentType.vbext_ct_Document
                    FolderName = Project.Name & ".Documents"
            End Select

            Component.CodeModule.InsertLines 1, Chr(39) & "@Folder(""" & FolderName & """)"
        End If
    Next
End Sub

Note: I purposely kept all the functionality in a single procedure for portability.
Kudos to the Rubberduck team the latest version Rubberduck VBA v2.5 is a C# WPF that follows the MVVM pattern to the letter!

HasFolder = InStr(.Lines(1, .CountOfLines), Chr(39) & "@Folder")

That condition can technically be True in modules that don't actually have the annotation; a @Folder Rubberduck annotation is only valid in a module's declarations section, so there's no need to grab the module content any further than .CountOfDeclarationLines - if the module is nearing the 10K lines capacity, using this instead of .CountOfLines could make a significant difference in the size of the string being passed to the InStr function.

Select Case Component.Type
    Case vbext_ComponentType.vbext_ct_StdModule
        FolderName = Project.Name & ".Modules"
    Case vbext_ComponentType.vbext_ct_ClassModule
        FolderName = Project.Name & ".Classes"
    Case vbext_ComponentType.vbext_ct_MSForm
        FolderName = Project.Name & ".Forms"
    Case vbext_ComponentType.vbext_ct_ActiveXDesigner
        FolderName = Project.Name & ".Designers"
    Case vbext_ComponentType.vbext_ct_Document
        FolderName = Project.Name & ".Documents"
End Select

I wouldn't repeat the concatenation here - just work out the last part of the name per the component type, and then concatenate with Project.Name & "." (I'd pull the separator dot out of the Case blocks as well) - and then I might give it a bit of breathing room, but that's more subjective:
    Select Case Component.Type
        Case vbext_ComponentType.vbext_ct_StdModule
            ChildFolderName = "Modules"

        Case vbext_ComponentType.vbext_ct_ClassModule
            ChildFolderName = "Classes"

        Case vbext_ComponentType.vbext_ct_MSForm
            ChildFolderName = "Forms"

        Case vbext_ComponentType.vbext_ct_ActiveXDesigner
            ChildFolderName = "Designers" 'note: not supported in VBA

        Case vbext_ComponentType.vbext_ct_Document
            ChildFolderName = "Documents"

    End Select
    FolderName = Project.Name & "." & ChildFolderName

I like the @Folder("Parent.Child") syntax and I see that's what you're generating here:

Component.CodeModule.InsertLines 1, Chr(39) & "@Folder(""" & FolderName & """)"

Note that this would also be legal... and simpler to generate:
Component.CodeModule.InsertLines 1, "'@Folder " & FolderName

Obviously if you prefer the parenthesized syntax (either works, it's really just down to personal preference) like I do then by all means keep it, but Rubberduck's new Move to Folder command doesn't put the parentheses in. I'd probably have the single-quote ' character spelled out, too, but I can see how a ' might be harder than necessary to read in the middle of a bunch of " double quotes. On the other hand, defining a constant for it would remove the need to have the "@Folder string literal defined in multiple places:
Private Const RD_FOLDER_ANNOTATION As String = "'@Folder "

...

Component.CodeModule.InsertLines 1, RD_FOLDER_ANNOTATION & FolderName

I have to mention that Rubberduck deliberately shoves all modules under the same default named-after-the-project folder (they can easily be sorted by component type in the Code Explorer), because we strongly believe grouping modules by component type is utterly useless and counter-productive: when I look at the code for a given functionality, I want to see all the code related to that functionality - and I couldn't care less about the component type of the code I'm looking at... it's mostly all class modules anyway.
A sane way to organize the modules in a project, is by functionality: you want your ThingView user form in the same place as your ThingModel class and your ThingPresenter and the Things custom collection - that way when you're working on that Thing, you don't have to dig up the various pieces in an ever-growing list of components under some useless "Class Modules" folder.


Copy and in-place transformation of strings to lowercase/uppercase I have the following code that transforms a string to upper/lowercase:

#include <iterator>
#include <algorithm>
#include <type_traits>
#include <vector>
#include <vector>
#include <iostream>
#include <string>
#include <locale>
#include <functional>

namespace ae
{
    template <typename StrT>
    auto toLower(StrT& str)
    {
        std::transform(str.begin(), str.end(), str.begin(), [](const auto& c)
        {
            static const auto loc{ std::locale() };
            return std::tolower(c, loc);
        });
    }

    template <typename StrT>
    auto toUpper(StrT& str)
    {
        std::transform(str.begin(), str.end(), str.begin(), [](const auto& c)
        {
            static const auto loc{ std::locale() };
            return std::toupper(c, loc);
        });
    }

    template <typename StrT>
    auto toLowerCopy(StrT& str)
    {
        auto newStr{ str };
        std::transform(str.begin(), str.end(), newStr.begin(), [](const auto& c)
        {
            static const auto loc{ std::locale() };
            return std::tolower(c, loc);
        });
        return newStr;
    }

    template <typename StrT>
    auto toUpperCopy(StrT& str)
    {
        auto newStr{ str };
        std::transform(str.begin(), str.end(), newStr.begin(), [](const auto& c)
        {
            static const auto loc{ std::locale() };
            return std::toupper(c, loc);
        });
        return newStr;
    }
}

// Example usage
int main()
{
    std::wstring my_str{ L"Test" };
    ae::toLower(my_str);
    std::wcout << my_str << std::endl;

    std::string my_str2{ "This is a test aswell" };
    auto new_str{ ae::toUpperCopy(my_str2) };
    std::cout << new_str << std::endl;

    return 0;
}

All of the 4 functions are pretty much exact copies of each other, just minor variations. I dont want to repeat myself in this way, but im unsure how to make one function that does all of this functionality. How can I improve the code?

What you can do is combine the two methods toLower and toUpper into one method, say transform, to which you pass the method by which to transform (std::tolower or std::toupper) as arguments. The same can be done for toUpperCopy and toLowerCopy. Passing functions allows you to be more flexible as you are now able to not only use std::tolower and std::toupper but also any other function which transforms a character.

You can combine everything into one method and use something like a boolean parameter to tell if it should copy or not but I would advise against it. The functions are short functions so they will still be readable and you keep the responsibility for copying in a separate function.

Passing the functions would look something like this:

namespace ae
{
    template <typename charT>
    auto transform(std::basic_string<charT>& str, charT charMap(charT, const std::locale&))
    {
        std::transform(str.begin(), str.end(), str.begin(), [&charMap](const auto& c)
        {
            static const auto loc{ std::locale() };
            return charMap(c, loc);
        });
    }

    template <typename charT>
    auto transformCopy(std::basic_string<charT>& str, charT charMap(charT, const std::locale&))
    {
        auto newStr{ str };
        std::transform(str.begin(), str.end(), newStr.begin(), [&charMap](const auto& c)
        {
            static const auto loc{ std::locale() };
            return charMap(c, loc);
        });
        return newStr;
    }
}

int main()
{
    std::wstring my_str{ L"Test" };
    ae::transform(my_str, std::tolower);
    std::wcout << my_str << std::endl;

    std::string my_str2{ "This is a test aswell" };
    auto new_str{ ae::transformCopy(my_str2, std::toupper) };
    std::cout << new_str << std::endl;

    return 0;
}

Note that I changed the template variable from StrT to charT as the charT was needed for the charMap. Using both StrT and charT, the compiler was not able to infer the template argument for charT so you had to manually add the template arguments each time you called ae::transform.


Converting file from Markdown-like markup into HTML using repeated substitutions The following code reads a file, splits its data, replaces some characters in the data, and then joins the data again (I added more details in the comments):

// read plain text file and make content available in data
fs.readFile(filename, 'utf8', function(err, data) {
  if (err) throw err

  // turn the data into an array
  data = data.split('\n\n')

  // make a clone of the array to be used in the if statements.
  var tree = data.slice()

  for (var i = 0; i < tree.length; ++i) {
    // turn #s into heading tags if #s are present
    if (tree[i].match(/^#/g)) {
      data[i] = data[i]
        .replace(/^#### (.*)/gm, '
$1
')
        .replace(/^### (.*)/gm, '
$1
')
        .replace(/^## (.*)/gm, '
$1
')
        .replace(/^# (.*)/gm, '
$1
')
    }

    // smarten " or ' if present
    if (tree[i].match(/"|'/g)) {
      data[i] = data[i]
        .replace(/"(?=\b|\*|')/g, '“')
        .replace(/"(?!\b|\*|')/g, '”')
        .replace(/'(?!\b|\*)|(?=\b)'(?=\b)/g, '’')
        .replace(/'(?=\b|\*)/g, '‘')
    }

    // turn -- into — if present
    if (tree[i].match(/--/g)) {
      data[i] = data[i]
        .replace(/\b--(\b)*/g, '—')
    }

    // turn * or ** into italics and bold if present
    if (tree[i].match(/\*\*|\*/g)) {
      data[i] = data[i]
        .replace(/\*\*([^\*|\s]+)\*\*/g, '$1')
        .replace(/\*([^\*|\s]+)\*/g, '$1')
    }

    // surround every element with p tags if the
    // element doesn't start with an #. Also if the previous element of
    // the element is # or * * * add the p tag with the class ni
    if (tree[i].match(/^[^#]/g)) {
      if (tree[i - 1] && (tree[i - 1].match(/^#/g) || tree[i - 1] === "* * *")) {
        data[i] = '
' + data[i] + '
'
      } else {
        data[i] = '
' + data[i] + '
'
      }
    }
  }

  // lastly, put the array together again to the saved as HTML
  data = data.join('\n\n')

  saveHtml(data)
})

Example input:

# Title

'Single quotes'

"Double Quotes"

* * *

Paragraphs

Output:

Title

‘Single quotes’

“Double Quotes”

* * *

Paragraphs

Is there a cleaner way to write those if statements? Or at least create a function so that there is less code in that fs.readFile block?

Here's what I would do (note: in ES6, though it is trivial to convert back to ES5). It doesn't match your code perfectly, but it should get get the point across. Essentially I extract everything out into smaller methods and then have a method that can perform batch replacements. It's not shorter nor really any less complex, but it is (to me) easier to read and follow.

Notes:

I'd normally comment this like crazy so that I could remember what the regexps do, but in this example I haven't. Almost all of them are your regexp's, and the ones that are different aren't so different that they won't be obvious to you.
I prefer HTML Entities for things like double quotes and dashes.
I'm not worrying about the p class="ni" stuff, but it would be trivial to add.

ES6 Code:

function massReplace(text, replacementArray) {
  let results = text;
  for (let [regex, replacement] of replacementArray) {
    results = results.replace(regex, replacement);
  }
  return results;
}

function transformHeadings(text, orig) {
  if (orig.match(/^#{1,6}\s/)) {
    return massReplace(text,
                       [ [/^###### (.*)/gm, '
$1
'],
                         [/^##### (.*)/gm,  '
$1
'],
                         [/^#### (.*)/gm,   '
$1
'],
                         [/^### (.*)/gm,    '
$1
'],
                         [/^## (.*)/gm,     '
$1
'],
                         [/^# (.*)/gm,      '
$1
'] ]
                      );
  }
}

function transformQuotes(text, orig) {
  if (orig.match(/"|'/)) {
    return massReplace(text,
                       [ [/"(?=\b|\*|')/g,             '&ldquo;'],
                         [/"(?!\b|\*|')/g,             '&rdquo;'],
                         [/'(?!\b|\*)|(?=\b)'(?=\b)/g, '&lsquo;'],
                         [/'(?=\b|\*)/g,               '&rsquo;'] ]
                      );
  }
}

function transformStyling(text, orig) {
  if (orig.match(/\*\*|\*/)) {
    return massReplace(text,
                       [ [ /\*\*([^\*|\s]+)\*\*/g, '$1'],
                         [ /\*([^\*|\s]+)\*/g,     '$1' ] ]);
  }
}

function transformDashes(text, orig) {
  if (orig.match(/\-\-/)) {
    return massReplace (text, [ [ /\-\-/g, '&mdash;' ] ]);
  }
}

function transformParagraphs(text, orig) {
  if (!orig.match(/^#{1,6} (.*)/)) {
    return `<p>${text}</p>`;
  }
}

function transformToHTML(markdownSource) {
  let data = markdownSource.split('\n\n'),
      orig = data.slice(),
      transforms = [ transformHeadings, transformQuotes, transformDashes,
                     transformStyling, transformParagraphs ];

  for (let i = 0, l = orig.length; i < l; ++i) {
    for (let transform of transforms) {
      let result;
      if ((result = transform(data[i], orig[i])) !== undefined) {
        data[i] = result;
      }
    }
  }

  return data.join('\n');
}

NOTE: For engines that don't support destructuring (looking at you, io.js), use this method instead:

function massReplace(text, replacementArray) {
  let results = text;
  for (let replacementArrayItem of replacementArray) {
    let regex = replacementArrayItem[0],
        replacement = replacementArrayItem[1];
    results = results.replace(regex, replacement);
  }
  return results;
}


Loops in PyTorch Implementation I'm trying to implement a regularization term for the loss function of a neural network.

from torch import nn
import torch
import numpy as np

reg_sig = torch.randn([32, 9, 5])
reg_adj = torch.randn([32, 9, 9, 4])

Maug = reg_adj.shape[0]

n_node = 9
n_bond_features = 4
n_atom_features = 5

SM_f = nn.Softmax(dim=2)
SM_W = nn.Softmax(dim=3)

p_f = SM_f(reg_sig)
p_W = SM_W(reg_adj)

Sig = nn.Sigmoid()

q = 1 - p_f[:, :, 4]
A = 1 - p_W[:, :, :, 0]

A_0 = torch.eye(n_node)
A_0 = A_0.reshape((1, n_node, n_node))
A_i = A

B = A_0.repeat(reg_sig.size(0), 1, 1)

for i in range(1, n_node):
    A_i = Sig(100 * (torch.bmm(A_i, A) - 0.5))

    B += A_i

C = Sig(100 * (B - 0.5))

reg_g_ij = torch.randn([reg_sig.size(0), n_node, n_node])

for i in range(n_node):
    for j in range(n_node):
        reg_g_ij[:, i, j] = q[:, i] * q[:, j] * (1 - C[:, i, j]) + (1 - q[:, i] * q[:, j]) * C[:, i, j]

I believe that my implementation is computationally not efficient and would like to have some suggestions on which parts I can change. Specifically, I would like to get rid of the loops and do them using matrix operations if possible. Any suggestions or working examples or links to useful torch functions would be appreciated

I don't have many improvements to offer-- just one major one. Like you suspected, your implementation is not efficient. This is because using a double for loop to set a Torch/NumPy array is not the preferred way to do sum reductions. What is preferred, is the use of torch.einsum. It takes an indices equation and reduces the Tensors into a final representation.

First to note is that your equation for reg_g_ij is not the most simplified form.

In your code, we start with:

q_i * q_j * (1 - C_ij) + (1 - q_i * q_j) * C_ij

But it can be reduced to:

q_i * q_j * (1 - 2 * C_ij) + C_ij

You can prove it yourself with a few lines of algebra.

The last small thing is call .unsqueeze(0) when you're expanding the dimensions of an array. In this case we used this method to expand an array's size from (9, 9) to (1, 9, 9).

    A_0 = torch.eye(n_node).unsqueeze(0)
    A_i = A

    B = A_0.repeat(reg_sig.size(0), 1, 1)

    for i in range(1, n_node):
        A_i = Sig(100 * (torch.bmm(A_i, A) - 0.5))

        B += A_i

    C = Sig(100 * (B - 0.5))

    reg_g_ij = torch.einsum('ij,ik,ijk->ijk', q, q, 1 - 2 * C) + C

When profiling this approach, we see a pretty big reduction in time:

In [257]: %timeit new(reg_sig, reg_adj)
1000 loops, best of 5: 745 µs per loop

In [258]: %timeit orig(reg_sig, reg_adj)
The slowest run took 4.85 times longer than the fastest. This could mean that an intermediate result is being cached.
100 loops, best of 5: 5.44 ms per loop


Finding odd and even digits from an integer I was given a task:

  Given any integer - write a program, which:

  finds all digits of the integer
  writes odd and even digits from the integer
  finds the biggest even and smallest odd digit
  writes the biggest possible number that can be formed by rearranging the odd digits
  writes the smallest possible number that can be formed by rearranging the even digits

I have written this code, but I am wondering if I can somehow get it written shorter than this:

#include <iostream>
#include <cmath>
using namespace std;
int main()
{
    long a,s=0,s1=0;
    int b,n=0,n1=0,k=0;
    int d[10],nl[10];
    int c=0;
    int i=0,j=0;
    for (i=0; i<10; i++)
    {
        d[i]=0;
        nl[i]=0;
    }
    cout<<"Write a number: "<<endl;
    cin>>a;
    i=0;
    while(a!=0) {
        b=a%10;
        a=a/10;
        if(b%2!=0) {
            d[i]=b;
            i++;
        }
        else {
            nl[k]=b;
            k++;
        }
    }
    n=i;
    n1=k;
    cout<<n1<<" even numbers "<<n<<" odd numbers "<<endl;
    cout<<" Odd numbers not ranged array"<<endl;
    for(i=0; i<n; i++)
        cout<<d[i]<<" ";
    cout<<endl;
    cout<<" Even numbers not ranged array"<<endl;
    for(i=0; i<n1; i++)
        cout<<nl[i]<<" ";
    cout<<endl;
    c=0;
    for(i=0; i<n; i++)
    {
        for(j=0; j<n; j++)
        {
            if(d[i]>d[j])
            {
                c=d[i];
                d[i]=d[j];
                d[j]=c;
            }
        }
    }
    cout<<" Odd numbers ranged array"<<endl;
    for(i=0; i<n; i++)
        cout<<d[i]<<" ";
    cout<<endl;
    cout<<"Min odd number "<<d[n-1]<<endl;
    c=0;
    for(i=0; i<n1; i++)
    {
        for(j=0; j<n1; j++)
        {
            if(nl[i]<nl[j])
            {
                c=nl[i];
                nl[i]=nl[j];
                nl[j]=c;
            }
        }
    }
    cout<<" Even numbers ranged array"<<endl;
    for(i=0; i<n1; i++)
        cout<<nl[i]<<" ";
    cout<<endl;
    cout<<"Max even number "<<nl[k-1]<<endl;
    for(i=0; i<n; i++)
    {
        s=s*10+d[i];
    }
    for(i=0; i<n1; i++)
        s1=s1*10+nl[i];
    cout<<"Max odd digits' number "<<s<<endl;
    cout<<"Min odd digits' number "<<s1<<endl;
    return 0;
}

Try to avoid using using namespace std.
<cmath> is not utilized anywhere, so just remove it unless you end up adding associated code.
Avoid giving variables single-letter names (unless they're simple loop counters, which is okay to do).  Although it may save some typing, it will instead make them appear meaningless as there is no context.  Comments can be added, but they're not a good substitute for actual variable names.  If a variable needs a comment, chances are it hasn't been given a good name.
To help increase readability and maintainability:

Have one variable per line.  This also helps in case comments need to be added.

long a;
long s=0;
long s1=0;

Separate operators and operands with whitespace so that the two can be identified.

long a;
long s = 0;
long s1 = 0;

Separate code lines based on purpose, also so that readers can focus on certain parts.

cout<<" Odd numbers ranged array"<<endl;

for(i=0; i<n; i++)
    cout<<d[i]<<" ";

cout<<endl;

Keep variables as close in scope as possible instead of listed together.  This could also help determine if a variable is no longer being used.

cout << "Write a number: " << endl;
long a; // move this here
cin >> a;

std::endl also flushes the stream, which is slower.  If you want just a newline, use "\n":

cout << "Write a number: \n";

This:

c=nl[i];
nl[i]=nl[j];
nl[j]=c;

can be done more concisely with std::swap:

std::swap(n1[i], n1[j]);

Consider making this program more modular (using functions).  Keeping everything in main() could make it hard for yourself and others to read and maintain the code.  It is also not clear, just from looking at it, that this program is written to fulfill those five tasks.  It could just be one task if the reader weren't otherwise aware of them.

This can be done any way you'd like, but the most likely solution could be to have one function for each of those tasks (while giving them appropriate names).  They could then be called from main(), and you could also get the user input from main() (this could, for instance, allow you to terminate the program right away if invalid input is given).


Java class that generates a short URL string for an input string Help me find out if the approach I took to generate random strings here is correct to be able to return the appropriate short URLs.

      import java.util.HashMap;
      import java.util.Map;
      import java.util.Map.Entry;
      import java.util.Random;
        /**
         *
         * Given a input string create a random
         * string
         *
         * @author
         * @creation Mar 23, 2015
         *
         */
        public class URLTinyMe {

    /*Map stores 1:1 mapping of input url and tinyurl*/
    private Map<String,String> urlMap = new HashMap<String,String>();

    private static URLTinyMe urlTinyMeObj = null;

    /**
     * return static instance of URLTinyMe
     * @return
     */
    public static URLTinyMe getInstance(){
      if(null == urlTinyMeObj){
        synchronized (URLTinyMe.class){
          if(null == urlTinyMeObj){
            urlTinyMeObj =  new URLTinyMe();

          }
      }
      }
      return urlTinyMeObj;
    }

    /*
     * Private constructor, restrict instance creation
     */
    private URLTinyMe() {

    }

    /**
     * TinyMefy a URL
     * @return String
     */
    public String getTinyMeUrl(String inputUrl){

        String tinyUrl = inputUrl;

        if(urlMap.containsKey(inputUrl)){//Check if url already exists
            tinyUrl =  urlMap.get(inputUrl);
        }else{
            StringBuilder charList = new StringBuilder();
            do{
                int random = getRandonNumber(48,122);//Accept only characters and numericals A-Z,0-9,a-z
                charList.append((char)random);
            }while(urlMap.containsValue(charList.toString()));

            urlMap.put(inputUrl, charList.toString());
            tinyUrl =  charList.toString();
        }
        return tinyUrl;
    }

    /**
     * From the tinyUrl return the original input url
     * @param tinyUrl
     * @return
     */
    public String getOriginalUrl(String tinyUrl){
      String inputUrl = null;
      if(urlMap.containsValue(tinyUrl)){
       for(Entry<String,String> entry:urlMap.entrySet()){
         if(entry.getValue().equals(tinyUrl)){
           inputUrl = entry.getKey();
         }
       }
      }
      return inputUrl;
    }
    /*
     * Return random number for the limit
     * */
    private int getRandonNumber(int lowLimit, int highLimit){
        int random = 0;
        Random randomObj = new Random();
        do{
            random = randomObj.nextInt(highLimit);

        }while(!(random >= lowLimit && random <= highLimit) ||
                (random > 57 && random < 65) ||
                (random > 90 && random <97));

        return random;
    }

}

Unit Tests and Single Responcibility

It's always good to test your code. With random data, this is a bit complicated, but at least your original mistake of the same input generating different urls would have been caught by tests.

Now the question becomes how to test that there will be no duplicates when randomness is involved. And the easy answer would be to remove the randomness from it.

As creating random numbers shouldn't be the responsibility of this class anyways (it's already responsible for managing urls), you could create a Random interface (with int getRandonNumber(int lowLimit, int highLimit) as method) whose instance you can inject into the class (via the getInstance(Random random) method). For unit tests, you can then simply create a dummy random object which returns a static value. Your test would then look something like this:

URLTinyMe url = URLTinyMe.getInstance((x, y) -> 60); // inject dummy random object that always returns 60
List<String> urls = new ArrayList<>();
for (int i = 0; i < 500; i++) {
    urls.add(url.getTinyMeUrl("foo" + i));
}
Assert.assertTrue(!hasDuplicates(urls));

Additional tests would be:

// same url creates same tiny url
Assert.assertEquals(url.getTinyMeUrl("foo"), url.getTinyMeUrl("foo"));

// getOriginalUrl works
String test = "foobar";
Assert.assertEquals(test, url.getOriginalUrl(url.getTinyMeUrl("foobar")));

Misc

Unnecessary assignments just complicate your code. String tinyUrl = inputUrl; isn't needed.
Your getRandonNumber doesn't do what it says it does, as it also filters out numbers in between the limits. Either change how the method works, or change the comment. I would probably get rid of the parameters, and rename the method to something that expresses that only alphnum is generated.
Your spacing and indentation is off (and not even internally consistent), which makes your code harder to read.
I don't think that you will gain any performance increase from using a StringBuilder if you call toString each time you append something.
You should probably throw an exception if the original url could not be found.


Coins problem in Erlang How many different ways can 200 be made using any number of coins? The possible denominations are 1, 2, 5, 10, 20, 50, 100, 200.

-module(coins).
-export([coins/2]).

coins(_, 0) -> 1;
coins([], _) -> 0;
coins(_, Goal) when Goal < 0 -> 0;
coins([H|T], Goal) ->
  coins([H|T], Goal-H) + coins(T, Goal).

The call is:

1> c(coins).
2> coins:coins([1, 2, 5, 10, 20, 50, 100, 200], 200).
73682

You can easily give more structure to your code:

-module(coins).
-export([coins/2]).

-spec coins([CoinValue::integer()], Goal::integer()) -> Combinations::integer().

coins(_, 0) -> 1;
coins([], _) -> 0;
coins(_, Goal) when Goal < 0 -> 0;
coins([CoinValue|Rest], Goal) ->
  WithCoinValue = coins([H|T], Goal-H),
  WithoutCoinValue = coins(T, Goal),
  WithCoinValue + WithoutCoinValue.


Calling different class objects using python Im trying to get more grip of Classes and also calling different classes to each other which will be more understandable when showing code before:
Utils.py
from proxymanager import ProxyManager # https://pypi.org/project/proxy-manager/

class ProxyServer:
    def __init__(self):
        self.proxy_manager_test = ProxyManager('./test.txt')
        self.proxy_manager_test2 = ProxyManager('./test2.txt')
        self.proxy_manager_test3 = ProxyManager('./test3.txt')
        self.proxy_manager_test4 = ProxyManager('./test4.txt')

    def hello(self):
        return self.proxy_manager_test.random_proxy().get_dict()

    def world(self):
        return self.proxy_manager_test2.random_proxy().get_dict()

    def hey(self):
        return self.proxy_manager_test3.random_proxy().get_dict()

    def yes(self):
        return self.proxy_manager_test4.random_proxy().get_dict()

class Testing:

    def GetData(self, value=None):
        if value:
           dataSaved = getattr(ProxyServer(), value)() # <--- Technically works but is it correct way to do it?
           print(dataSaved)

Main.py

import random
import time
from .utils import Testing

scraper = Testing()

randomList = ["hello", "world", "hey", "yes"]

while True:
    response = scraper.GetData(
        value=random.choice(randomList)
    )
    time.sleep(10)

My concern is the dataSaved = getattr(ProxyServer(), value)() which seem to work as expected but i'm not sure if its correct way to do it, I did get some review about it but the answer I got was that it will technically work but its wrong to do it this way and I want a second opinion from you guys, Is it actually wrong doing it this way?
And please, let me know if I am missing anything else here to provide missing information :)

I think you've over-engineered the code a bit. If I were to write this code I wouldn't have used a class at all. Looking at what your code does I understand that:

you have multiple proxies files
you randomly choose one of them and perform whatever actions based on that

Why not just doing something along the following lines:
import random
import time

from proxymanager import ProxyManager

PROXY_FILES = (
    './test.txt',
    './test2.txt',
    './test3.txt',
    './test4.txt',
)
DELAY = 10

def get_random_proxy():
    """
    Return a random proxy dict from a random file.
    """
    random_file = random.choice(PROXY_FILES)
    return ProxyManager(random_file).random_proxy().get_dict()

def main():
    while True:
        proxy = get_random_proxy()
        print(f'Do something with {proxy["http"]}\n')
        time.sleep(DELAY)

if __name__ == '__main__':
    main()

This approach will save you a lot of development time since you:

won't have to add a new method to your ProxyServer each time you have a new proxy file.
won't have to keep track of random_list (which is not quite random since it has to match the attributes of the class)
you won't need a Testing() class anymore

Notice that with this version is really easy to figure out what you're doing.
Recommendations (PEP8):

use appropriate naming for your variables (snake_case naming convention for methods/functions, UPPER_CASE naming convention for constants);
use docstrings to help your readers understand the code;

If you're really tied to use classes, I'd mix your solution with mine and do something along these lines:
import random
import time

import requests

from proxymanager import ProxyManager

PROXY_FILES = (
    './test.txt',
    './test2.txt',
    './test3.txt',
    './test4.txt',
)
DELAY = 10

class ProxyServer:
    def __init__(self, random_proxy_file):
        self.proxy_manager = ProxyManager(random_proxy_file)

    def get_proxy(self):
        """
        Return a random proxy dict.
        """
        return self.proxy_manager.random_proxy().get_dict()

class Scrapper:
    def __init__(self, base_url):
        self.base_url = base_url

    @staticmethod
    def _get_random_proxy():
        random_proxy_file = random.choice(PROXY_FILES)
        return ProxyServer(random_proxy_file).get_proxy()

    def get_url_data(self):
        proxy = self._get_random_proxy()
        data = requests.get(self.base_url, proxies={'http': proxy['http']})
        return data

    def do_something_with_data(self, data):
        return data

def main():
    scrapper = Scrapper('http://scrapeme.not/')

    while True:
        data = scrapper.get_url_data()
        scrapper.do_something_with_data(data)
        time.sleep(DELAY)

if __name__ == '__main__':
    main()

But notice how this has become a lot more complex than it should've been. Some advantages that this has over your solution:

it uses better naming;
it avoids creating multiple ProxyManager() instances;
it avoids mapping the getattr() to a value from a list you'd have to maintain all the time as complexity of the scrapper increases;

And to answer your question: no, it's not wrong to use getattr() like that but in this specific case I don't think it's necessary due to the above listed reasons.


Implementing delegate methods in an UIView class This is my UIView class:

class OnboardingView: UIView {

    @IBOutlet weak var scrollView: UIScrollView!

    @IBOutlet weak var skipButton: UIButton!
    @IBOutlet weak var nextButton: UIButton!
    @IBOutlet weak var pageControl: UIPageControl!

    @IBOutlet weak var skipButtonTopConstraint: NSLayoutConstraint!
    @IBOutlet weak var nextButtonTopConstraint: NSLayoutConstraint!
    @IBOutlet weak var pageControlBottomConstraint: NSLayoutConstraint!

    fileprivate var stepviews: [OnboardingStepViewDelegate] = []

    weak var delegate: OnboardingDelegate?

    fileprivate var visibleIndex: Int = 0 {
        willSet {
                stepviews[visibleIndex].didDisappear()
        }
        didSet {
            stepviews[visibleIndex].didAppear()
        }
    }

    override func awakeFromNib() {
        super.awakeFromNib()

        scrollView.contentSize = CGSize(width: scrollView.frame.width * CGFloat(stepviews.count), height:scrollView.frame.height)
        scrollView.delegate = self

        let firstPage = OnboardingIntroductionStepView.instanceFromNib(title: "Some title", message: "Message", imageName: "page1")

        stepviews = [firstPage, second, third]

        scrollView.contentSize = CGSize(width: 3 * scrollView.frame.size.width, height: scrollView.frame.size.height)

        scrollView.addSubview(firstPage)
    }
}

private extension OnboardingView {

    @IBAction func nextPage(_ sender: Any) {
        if pageControl.currentPage == stepviews.endIndex{
            return
        }

        let nextPageIndex = CGFloat(pageControl.currentPage + 1)

        UIView.animate(withDuration: 0.3) {
            self.scrollView.contentOffset.x = self.scrollView.frame.width * nextPageIndex
        }

        pageControl.currentPage += 1
    }

    @IBAction func skip(_ sender: Any) {
        pageControl.currentPage = stepviews.endIndex
        nextPage(self)
    }

    func moveControlConstraintsOffScreen () {
        pageControlBottomConstraint.constant = 40
        skipButtonTopConstraint.constant = -40
        nextButtonTopConstraint.constant = -40
    }

    func moveControlConstraintsOnScreen () {
        pageControlBottomConstraint.constant = 0
        skipButtonTopConstraint.constant = 16
        nextButtonTopConstraint.constant = 16
    }
}

extension OnboardingView: UIScrollViewDelegate{
    func scrollViewDidScroll(_ scrollView: UIScrollView) {
        self.endEditing(true)

        visibleIndex = Int(scrollView.contentOffset.x / self.frame.width)

        delegate.doSomething()
    }

    func scrollViewWillEndDragging(_ scrollView: UIScrollView, withVelocity velocity: CGPoint, targetContentOffset: UnsafeMutablePointer<CGPoint>) {
        let pageNumber = Int(targetContentOffset.pointee.x / self.frame.width)
        pageControl.currentPage = pageNumber
    }
}

As you can see my view implements the UIScrollViewDelegate. However, this delegate method only handles view related logic. After this view logic is done it will notify the delegate (My ViewController) to do something. Using this approach my ViewController will only contain lifecycle updates (ViewDidLoad, ViewWillDissapear) etc. This makes sense in my opinion - This view class contains a scrollview that implements its logic to do view related tasks, once those tasks are done it tells the VC what to do based on delegation.

My question is: Is this bad and if so, why? Normally MVC is used as the design pattern, which results in "God" ViewControllers where ViewControllers also implement Datasource and Delegate. This results in a ViewController that handles controller logic AND view logic, making the Controller AND View less reusable.

I personally separate my code in my view controllers using the comment //MARK: - which adds shortcuts to your code in the top navigation bar (the one that shows what file you're currently at). I would keep @IB related properties in your original view controller rather than make a extension for the @IBActions.

Also, making an extension for delegates is good practice because it helps separate your code from Apple's delegate methods. I don't see anything wrong with this approach since you can either make an extension to your view, or add it within the original class declaration, but as far as MVC goes, this would be the ideal approach.

As long as you're not doing heavy data/logic/algorithms going on, then you're good.

as for the delegate.doSomething() call, I don't see the point of using delegate since the scroll view delegate code is in an extension of the view, which means doSomething() is already accessible.


User class for future inheritance in other programs involving user activity I am making an user class which I could inherit and use in the future.
(recently started learning Python and OOP)

import mysql.connector as mycon
class user:
    host = "localhost"
    user = "root"
    password = "pass"
    database = "USERS"
    table = "UserData"

    def __init__(self,firstName,lastName,userName,email):
        self.firstName = firstName
        self.lastName = lastName
        self.userName = userName
        self.fullName = firstName + " " + lastName
        self.email = email

    @classmethod
    def login(cls,userName,password):
        db = mycon.connect(host = cls.host,user = cls.user,password = cls.password,database = cls.database)
        cursor = db.cursor()
        r=cursor.execute("SELECT * FROM {} WHERE UserName = '{}';".format(cls.table,userName))
        userInfo = cursor.fetchone()
        if(userInfo!=None):
            if(userInfo[2]==password):
                print("Login Successful")
                return cls(userInfo[0],userInfo[1],userInfo[3],userInfo[4])
            else:
                print("INVALID PASSWORD",password,userInfo[2])
        else:
            print("USER DOES NOT EXIST")
        db.close()

    @classmethod
    def register(cls,firstName,lastName,password,userName,email):
        db = mycon.connect(host = cls.host,user = cls.user,password = cls.password,database = cls.database)
        cursor = db.cursor()
        r=cursor.execute("Insert into {} values('{}','{}','{}','{}','{}');".format(cls.table,firstName,lastName,password,userName,email))
        db.commit()
        db.close()

with a table UserData with description

One of the most important things to take care of when writing a database application is to prevent unwanted access. This means that you always want to make sure that there is no user (and the programmer, i.e. you should be included in that) can supply an input that can escape the query and e.g. delete your whole database. This attack vector is called SQL injection and is very common. Avoiding it is relatively easy, thankfully, although you cannot do it for the table name:

@classmethod
def login(cls, user_name, password):
    db = mycon.connect(host=cls.host, user=cls.user, password=cls.password,
                       database=cls.database)
    cursor = db.cursor()
    query = f"SELECT firstName, lastName, password, UserName, email FROM {cls.table} WHERE UserName = %s;"
    r = cursor.execute(query, (user_name,))
    user_info = cursor.fetchone()
    if user_info is not None:
        if user_info[2] == password:
            print("Login Successful")
            return cls(user_info[0],user_info[1],user_info[3],user_info[4])
        else:
            print(f"INVALID PASSWORD {password} {user_info[2]}")
    else:
        print("USER DOES NOT EXIST")
    db.close()

Note that I followed Python's official style-guide, PEP8, which recommends using lower_case for variables and functions, spaces after commas and around = when using it as an assignment, but no space around the = when using it for keyword arguments.

Comparisons to None should always be done with is or is not and if conditions don't require parenthesis, since it is a keyword.

I also explicitly named the column names (they might be called differently, judging by UserName, so you would have to fix that), to ensure that you can change the table structure, as long as these names stay the same.

And I used an f-string for easy formatting in case of an invalid password. Note that in this case you do reveal the actual password (I guess this is just for testing, just make sure you don't forget to remove that information later). In a similar vein, you should not store passwords in plaintext. Hash them with a good hashing algorithm and ideally add a salt.

You could also use the mysql.cursor.MySQLCursorNamedTuple cursor, so you can say user_info.password instead of user_info[2]. Just use cursor = db.cursor(named_tuple=True).

You should do yourself a favor and add a docstring to every method you write, to explain what the method does, what its arguments are and what it returns.


Iterative solution for generating all permutations of a string I know the runtime for generating all permutations is supposed to be \$O(n!)\$. I'm not sure if the code I wrote for it runs in \$O(n!)\$ though. I'm having trouble analyzing its runtime.

The for(char c: cArray) part is in \$O(n)\$. What about for (String s: currResults) and for (int i = 0; i < s.length() + 1; i++)?

public static Set<String> getPermutations(String str) {
        char[] cArray = str.toCharArray();
        Set<String> realRes = new HashSet<String>();
        Set<String> results = new HashSet<String>();
        for (char c : cArray) {
            Set<String> currResults = new HashSet<String>(results);
            String charToAdd = String.valueOf(c);
            results.add(charToAdd);
            for (String s : currResults) {
                for (int i = 0; i < s.length() + 1; i++) {
                    String newWord = s.substring(0, i) + charToAdd + s.substring(i);
                    if (newWord.length() == str.length()) {
                        realRes.add(newWord);
                    }
                    else {
                        results.add(newWord);
                    }
                }
            }
        }
        return realRes;
    }

Time complexity analysis

Let's think about what this does.

        for (char c : cArray) {

This says that for each character in the string (since cArray is the character array view of the string), we iterate once.  As you note, this is \$O(n)\$ where \$n\$ is the number of characters in the string.

        Set<String> currResults = new HashSet<String>(results);

This is \$O(m)\$ where \$m\$ is the number of entries in the Set.  Note that that will change as you go.

        String charToAdd = String.valueOf(c);
        results.add(charToAdd);

This takes constant time.

        for (String s : currResults) {

This takes \$O(m)\$ time again.

            for (int i = 0; i < s.length() + 1; i++) {

This takes \$O(l)\$ time, where \$l\$ is the length of this string.  Again, this will change over time.

                String newWord = s.substring(0, i) + charToAdd + s.substring(i);

This takes \$O(l)\$ time, as it involves copying s to a new string.

                if (newWord.length() == str.length()) {
                    realRes.add(newWord);
                }
                else {
                    results.add(newWord);
                }

This takes constant time.

We do \$n\$ iterations of the outer loop.

First iteration

\$n\$ is the number of characters in str and will be throughout.

\$m\$ is 0.  We haven't put any entries in the set yet.

\$l\$ doesn't exist.  We never get to that section of code.

We end with one entry in results and no entries in realRes.

Second iteration

\$m\$ is 1.

\$l\$ is 1.

We end with three entries in results and no entries in realRes.

Third iteration

\$m\$ is 3.  This is 2 + 1.

\$l\$ ranges from 2 to 1.

We end with nine entries in results and no entries in realRes.

Fourth iteration

\$m\$ is 9.  This is 3 * 2 + 2 + 1.

\$l\$ ranges from 3 to 1.

We end with forty entries in results and no entries in realRes.

nth iteration

We end with \$n!\$ entries in realRes.  We add increasing numbers of strings to results, equal to \$\sum\limits_{i=1}^{n-1} i!\$.  Note that that is less than \$n!\$.

Note that this assumes that all the characters in the string are different.  If some characters repeat, this will exaggerate the number of entries.  Of course, for Big-O notation, that's fine.  So we can say that overall this is still \$O(n!)\$.

Alternately styled code

    public static Set<String> getPermutations(String str) {
        Set<String> results = new HashSet<>();
        Set<String> partials = new HashSet<>();
        for (char c : str.toCharArray()) {
            List<String> current = new ArrayList<>();
            String charToAdd = String.valueOf(c);
            for (String s : partials) {
                for (int i = 0; i <= s.length(); i++) {
                    String newWord = s.substring(0, i) + charToAdd + s.substring(i);
                    if (newWord.length() == str.length()) {
                        results.add(newWord);
                    } else {
                        current.add(newWord);
                    }
                }
            }

            partials.add(charToAdd);
            partials.addAll(current);
        }

        return results;
    }

I don't like the name results for what you had.  It's not results.  It's partial permuations.  So I changed it to partials.

With the name results available, I'd prefer that to realRes.

I find it easier to read if we do the toCharArray in the for loop itself.  That way I don't have to look around to find what cArray is.

Rather than copy the entire set to another set on every iteration, I'd rather just copy the things that I'm adding.  So I made a List for that purpose.  That (and moving the charToAdd add) allows us to loop directly over partials rather than the new collection.

We can save a math operation if we say i <= s.length() rather than i < s.length() + 1.  I also find it a bit clearer about what it is doing.

The Java standard is to do the fully cuddled else.  It's not a big deal if you do it the other way, but I changed that on my way through.


How to run canvas animation without effecting other javascript code It seems to be the case that the canvas animation affects other JavaScript functions when I use window.onload = function() {. Please let me know the best practice to use animation without affecting other JavaScript.

window.onload = function() {
  //canvas init
  var canvas = document.getElementById("snow");
  var ctx = canvas.getContext("2d");

  //canvas dimensions
  var W = window.innerWidth;
  var H = window.innerHeight;
  canvas.width = W;
  canvas.height = H;

  //snowflake particles
  var mp = 250; //max particles
  var particles = [];
  for (var i = 0; i < mp; i++) {
    particles.push({
      x: Math.random() * W, //x-coordinate
      y: Math.random() * H, //y-coordinate
      r: Math.random() * 4 + 1, //radius
      d: Math.random() * mp //density
    });
  }

  //Lets draw the flakes
  function draw() {
    ctx.clearRect(0, 0, W, H);

    ctx.fillStyle = "rgba(255, 255, 255, 0.8)";
    ctx.beginPath();
    for (var i = 0; i < mp; i++) {
      var p = particles[i];
      ctx.moveTo(p.x, p.y);
      ctx.arc(p.x, p.y, p.r, 0, Math.PI * 2, true);
    }
    ctx.fill();
    update();
  }

  //Function to move the snowflakes
  //angle will be an ongoing incremental flag. Sin and Cos functions will be applied to it to create vertical and horizontal movements of the flakes
  var angle = 0;

  function update() {
    angle += 0.01;
    for (var i = 0; i < mp; i++) {
      var p = particles[i];
      //Updating X and Y coordinates
      //We will add 1 to the cos function to prevent negative values which will lead flakes to move upwards
      //Every particle has its own density which can be used to make the downward movement different for each flake
      //Lets make it more random by adding in the radius
      p.y += Math.cos(angle + p.d) + 1 + p.r / 2;
      p.x += Math.sin(angle) * 2;

      //Sending flakes back from the top when it exits
      //Lets make it a bit more organic and let flakes enter from the left and right also.
      if (p.x > W + 5 || p.x < -5 || p.y > H) {
        if (i % 3 > 0) {
          //66.67% of the flakes
          particles[i] = {
            x: Math.random() * W,
            y: -10,
            r: p.r,
            d: p.d
          };
        } else {
          //If the flake is exitting from the right
          if (Math.sin(angle) > 0) {
            //Enter from the left
            particles[i] = {
              x: -5,
              y: Math.random() * H,
              r: p.r,
              d: p.d
            };
          } else {
            //Enter from the right
            particles[i] = {
              x: W + 5,
              y: Math.random() * H,
              r: p.r,
              d: p.d
            };
          }
        }
      }
    }
  }

  //animation loop
  setInterval(draw, 33);
};
body {
  background: tomato;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<canvas id="snow" style="display:block;position:fixed;z-index:999;pointer-events:none;"></canvas>

One solution to allow multiple functions called when the page loads is to add a callback using EventTarget.addEventListener() for the DOMContentLoaded event.

window.addEventListener('DOMContentLoaded', function() {
    //canvas init
    //..
});

One could also simply use the load event. Many believe that modern browsers typically won't have DOM elements not query-able via JavaScript- especially if the script tag is loaded in the <body> tag instead of the <head> tag, and thus one wouldn't need to wait for the DOMContentLoaded event.

window.addEventListener('DOMContentLoaded', function() {
  //canvas init
  var canvas = document.getElementById("snow");
  var ctx = canvas.getContext("2d");

  //canvas dimensions
  var W = window.innerWidth;
  var H = window.innerHeight;
  canvas.width = W;
  canvas.height = H;

  //snowflake particles
  var mp = 250; //max particles
  var particles = [];
  for (var i = 0; i < mp; i++) {
    particles.push({
      x: Math.random() * W, //x-coordinate
      y: Math.random() * H, //y-coordinate
      r: Math.random() * 4 + 1, //radius
      d: Math.random() * mp //density
    });
  }

  //Lets draw the flakes
  function draw() {
    ctx.clearRect(0, 0, W, H);

    ctx.fillStyle = "rgba(255, 255, 255, 0.8)";
    ctx.beginPath();
    for (var i = 0; i < mp; i++) {
      var p = particles[i];
      ctx.moveTo(p.x, p.y);
      ctx.arc(p.x, p.y, p.r, 0, Math.PI * 2, true);
    }
    ctx.fill();
    update();
  }

  //Function to move the snowflakes
  //angle will be an ongoing incremental flag. Sin and Cos functions will be applied to it to create vertical and horizontal movements of the flakes
  var angle = 0;

  function update() {
    angle += 0.01;
    for (var i = 0; i < mp; i++) {
      var p = particles[i];
      //Updating X and Y coordinates
      //We will add 1 to the cos function to prevent negative values which will lead flakes to move upwards
      //Every particle has its own density which can be used to make the downward movement different for each flake
      //Lets make it more random by adding in the radius
      p.y += Math.cos(angle + p.d) + 1 + p.r / 2;
      p.x += Math.sin(angle) * 2;

      //Sending flakes back from the top when it exits
      //Lets make it a bit more organic and let flakes enter from the left and right also.
      if (p.x > W + 5 || p.x < -5 || p.y > H) {
        if (i % 3 > 0) {
          //66.67% of the flakes
          particles[i] = {
            x: Math.random() * W,
            y: -10,
            r: p.r,
            d: p.d
          };
        } else {
          //If the flake is exitting from the right
          if (Math.sin(angle) > 0) {
            //Enter from the left
            particles[i] = {
              x: -5,
              y: Math.random() * H,
              r: p.r,
              d: p.d
            };
          } else {
            //Enter from the right
            particles[i] = {
              x: W + 5,
              y: Math.random() * H,
              r: p.r,
              d: p.d
            };
          }
        }
      }
    }
  }

  //animation loop
  setInterval(draw, 33);
});
window.addEventListener('DOMContentLoaded', function() {
  console.log('another onload callback');
});
body {
  background: tomato;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<canvas id="snow" style="display:block;position:fixed;z-index:999;pointer-events:none;"></canvas>


Python code for serving k customers with bounded time window As I asked a question here, I want to write a python code for the following problem:
A person provides a service and he/she can serve \$k\$ clients each minute. Now, client number \$i\$ comes at the beginning of minute \$ a_{i} \$ and waits \$w_{i}\$ minutes to receive the service and if they don't receive the service in the interval \$[a_{i}, a_{i} + w_{i}]\$, they leave. If we have \$n\$ clients, can the person serve all them or not?
Now, by the answer stated there, I wrote the following code.
intervals = [] # Some intervals like [[1, 2], [1, 100], [1, 1], [5, 20]]

intervals = sorted(intervals)

start = intervals[0][0]
end = intervals[-1][1]

i = start
while i < end:
    if not intervals:
        break
    else:
        for j in range(k):
            try:
                if intervals[0][0] <= i <= intervals[0][1]:
                    del intervals[0]
                else:
                    break
            except:
                pass
    i += 1

if not intervals:
    print('YES')
else:
    print('NO')

My question: Is my code right? Can it become more efficient?

Time complexity is unnecessarily large.
First, it is linear in the timespan of the arrivals. A list [[1,2], [100,101]] would require 100 iterations; a list [1,2], [1000,1001]] would require 1000 iterations, etc. Don't bruteforce every minute.
Second, del intervals[0] has a linear time complexity in number of intervals, which results in an overall quadratic time complexity. A cheap way out is to sort in a descending order, and work right to left. This way you would remove intervals from the tail of the list, which is O(1).

DRY. A presence of two semantically identical breaks (the loop is broken when it runs out of the intervals) suggests refactoring. Do not test for if not intervals explicitly. Consider
  try:
      while i < end:
          for j in range(k):
              if intervals[0][0] <=i <= intervals[0][1]:
                  del intervals[0]
          i += 1
  except:
      pass

Of course, never except blindly. Catch only exceptions you expect and know how to handle, in this case except IndexError:.


JavaScript group anagram performance I created an algorithm to create lists of anagrams
The input is ["eat", "tea", "tan", "ate", "nat", "bat"]
Then, the output should be:
[["eat","tea","ate"],["tan","nat"],["bat"]]
Each array is words that are anagram.
My solution is:
function group(input) {
    let result = [];
    if(input.length === 0){
        return result;
    }

    for(let i=0; i<input.length; i++){
        let check = true;
        for(let j=0; j<result.length; j++){
            if(result[j].includes(input[i])){
                check = false;
            }
        }
        if(check){
            result.push(checkAnagrams(input, i));
        }
    }

    return result;
}

function checkAnagrams(aStr, i){
    let anagrams = [];
    anagrams.push(aStr[i]);
    for(let j=0; j<aStr.length; j++){
        if(i !== j){
            if(isAnagram(aStr[i], aStr[j])){
                anagrams.push(aStr[j]);
            }
        }
    }
    return anagrams;
}

function isAnagram(str1, str2) {
    // split => string into array
    // sort => sort the array
    // join => string to array
    // trim => remove any space
    const string1 = str1.split("").sort().join().trim();
    const string2 = str2.split("").sort().join().trim();
    if(string1 === string2){
        return true;
    }
    return false;
}

I know it works, but I was wondering if there is a better way to implement it.
Thanks

Welcome to Code Review, the complexity of your code can be reduced using a Map object having as keys sorted lexicographically strings and as values arrays of their anagrams. For example you will have the following couples key - value starting from your input:
key = "aet"  value = ["eat","tea","ate"]
key = "ant"  value = ["tan","nat"]
key = "abt"  value = ["bat"]

So you can define a function anagrams like this:
function anagrams(input) {
    const map = new Map();

    for (let i = 0; i < input.length; ++i) {
        const key = [...input[i]].sort().join('');
        const value = map.has(key) ? map.get(key) : [];
        value.push(input[i]);
        map.set(key, value);
    }

    return [...map.values()];
}

/*below it will print the expected result*/
console.log(anagrams(["eat", "tea", "tan", "ate", "nat", "bat"]));

I just used the Spread operator to expand the string input[i] to array of characters and after I sort it to obtain the corrisponding key in the map. After I add it to the array of strings associated to the key and finally I return the map values as an array.
Note: I'm a javascript beginner so every hint or criticism about my answer is highly appreciated.