Save points cloud in PLOT3D format I have the following function to save points cloud in PLOT3D format:

def write(filename, mesh):
    """
    Write the mesh to filename in PLOT3D format.
    filename: is the name of the output file.
    mesh: is a numpy array of size (N,2) or (N, 3) where N is the number of points. mesh[i] is a numpy array coordinates of the i'th point.

    """
    points = mesh.points
    imax = np.unique(points[:, 0]).size
    jmax = np.unique(points[:, 1]).size

    _2d = False
    if points.shape[1] == 2:
        _2d = True
    elif np.unique(points[:, 2]).size == 1:
        _2d = True

    with open(filename, "w") as p3dfile:
        if not _2d:
            kmax = np.unique(points[:, 2]).size
            print(imax, jmax, kmax, file=p3dfile)
            for value in points.flatten(order="F"):
                print(value, file=p3dfile)
        else:
            print(imax, jmax, file=p3dfile)
            for value in points[:, 0:2].flatten(order="F"):
                print(value, file=p3dfile)

For large number of points this function is very slow.

I appreciate any suggestion to improve the code above.

Optimizations/improvements:

Namings

write is too generic name for specific function. The one of better names is write_points
_2d. Even if a variable is marked as "protected" _..., starting its name with a digit is a bad naming pattern. As it's a boolean flag a meaningful name would be is_2dim (is 2-dimensional space)

Since mesh argument is only used to access its inner property points it's better to pass the target points data structure directly (It's called Replace Query with Parameter technique)

In current approach, when reaching the condition if not _2d: the expression np.unique(points[:, 2]).size will be calculated repeatedly, instead:
since points array is expected to be 2d array with either 2 or 3 columns we can collect unique counts across all columns at once (instead of declaring imax, jmax):

col_counts = [np.unique(row).size for row in points.T]

The whole conditional:

_2d = False
if points.shape[1] == 2:
    _2d = True
elif np.unique(points[:, 2]).size == 1:
    _2d = True

is now replaced with a single statement:

is_2dim = points.shape[1] == 2 or col_counts[2] == 1

Calling print function at each iteration to write the data into file:

for value in points.flatten(order="F"):
    print(value, file=p3dfile)

is definitely less efficient and performant than calling writelines() on generator expression at once.

The final optimized function:

def write_points(filename, points):
    """
    Write the mesh points to filename in PLOT3D format.
    filename: is the name of the output file.
    mesh: is a numpy array of size (N,2) or (N, 3) where N is the number of points. mesh[i] is a numpy array coordinates of the i'th point.

    """
    col_counts = [np.unique(row).size for row in points.T]
    is_2dim = points.shape[1] == 2 or col_counts[2] == 1

    with open(filename, "w") as p3dfile:
        pd3file.write(' '.join(map(str, col_counts)))
        pd3file.writelines(f'{num}\n' for num in points.flatten(order="F"))


Create matrices of a certain format I need to create matrices that follow a specific format: the matrix is a square NxN matrix and the off-diagonals all have the same value (param) and the diagonals have values such that the rows sum to zero (in other words, they are equal to 0 - param * (N-1). The output is a numpy array

Here is my code. Is there any way I can make this more readable/elegant? Speed is also important.

import numpy as np

def makeMatrix(param, N):
    ar = []
    for i in range(N):
        ar.append([ param for st in range(N) ])
        ar[i][i] = 0-(param*(N-1))

    return np.array(ar, dtype=np.double)

One of the main features of NumPy is that you can work with matrices without explicit looping.  To get the diagonal, use np.identity().  To set everything else, use broadcasting.

def make_matrix(value, dim):
    return value - value * dim * np.identity(dim, dtype=np.double)

param and N don't mean anything to me.  I suggest value and dim.  The function name should preferably be make_matrix, according to PEP 8.


Handling parsing failure in Scala without exceptions I have a Scala (Play!) application that must get and parse some data in JSON from an external service. I want to be able to gently handle failure in the response format, but it is becoming messy. What I am doing right now is, for instance,

def columnsFor(json: JsValue): Option[Map[String, String]] =
  try {
    val cols = (json \ "responseBody" \ "columns") match {
      case JsArray(xs) => xs map { col =>
        ((col \ "leafid").as[String], (col \ "name").as[String])
      }
      case _ => throw new Exception("Response unparsable")
    }
    Some(cols.toMap)
  }
  catch {
    case e: Exception => None
  }

That is, I go parsing optimistically and throw an exception whenever I find something that does not match my expected format (the methods as[String] also raise exceptions when they do not find strings). Then I recover from the exception and give back a None.

This approach is working and allows me to go from a potentially broken textual response to some type-safe data.

The problem is that having all these try..catch blocks looks ugly and messy. In theory it would be nicer to use optional types during all parsing steps. But then all my data structures become messy, as at each level you must have Options. When the data is deeply nested, I do not know how to handle failure at all levels. Moreover, I do not want to return something like an Option[Map[Option[String], Option[String]], but just an Option[Map[String, String]].

Is there some better/more idiomatic way to handle failures during parsing?

EDIT

I try to make the question more clear. Say I am expecting a response like

{
    "foo": [1, 2, 3],
    "bar": [2, 5]
}

I want to be able to parse it; taking failure into account I want to get something of type Option[Map[String, List[Int]]]. Now the problem is that parsing errors can appear at any level. Maybe I get a map with string keys, but its values are not lists. Or maybe the values are lists, but the content of the lists are strings instead of ints. And so on.

Whenever I want to parse a value, say as a string, I can choose between x.as[String] and x.asOpt[String]. The former will throw an exception when dealing with wrong input, while the latter returns a Option[String]. Similarly, when dealing with an array, I can do pattern matching like

x match {
  case JsArray(xs) => // continue parsing
  case _ => // deal with failure
}

I can deal with failure returning None or with an exception.

If I choose to use exceptions all along, then I can just wrap everything inside a try..catch and be sure to have handled all failures. Otherwise, my problem is that at any level, I have to put an option. So I have to

get something with an awful type like Option[Map[Option[String], Option[List[Option[Int]]]]]
find some way to flatten this all the way down to turn into the more manageable Option[Map[String, List[Int]]]

Of course, thing become even worse as soon as the JSON gets more nested.

Having monads in your data structures is not messy, they allow you always to flatten their content during nested calculation steps. A simple definition of them can be:

trait Monad[A] {
  def map[B](f: A => B): Monad[B]
  def flatMap[B](f: A => Monad[B]): Monad[B]
}

Because Option is a monad you do not have to work with nested types:

scala> def inc(i: Int) = Option(i+1)
inc: (i: Int)Option[Int]

scala> val opt = inc(0)
opt: Option[Int] = Some(1)

scala> val nested = opt map inc
nested: Option[Option[Int]] = Some(Some(2))

scala> val flattened = opt flatMap inc
flattened: Option[Int] = Some(2)

Thus, there should never be a reason to nest monads deeply. Scala also has the for-comprehension which automatically  decomposes monads:

scala> :paste
// Entering paste mode (ctrl-D to finish)

val none = for {
  o1 <- inc(0)
  o2 <- inc(o1)
  o3 <- inc(o2)
} yield o3

val some = for {
  o1 <- inc(0)
  o2 <- inc(o1)
  o3 <- inc(o2-1)
} yield o3

// Exiting paste mode, now interpreting.

none: Option[Int] = None
some: Option[Int] = Some(2)

In order to work with exceptions scala.util.Try is introduced in 2.10:

scala> import scala.util.{Try, Success, Failure}
import scala.util.{Try, Success, Failure}

scala> def err: Int = throw new RuntimeException
err: Int

scala> val fail = Try{err} map (_+1)
fail: scala.util.Try[Int] = Failure(java.lang.RuntimeException)

scala> val succ = Try{0} map (_+1)
succ: scala.util.Try[Int] = Success(1)

Nevertheless, Try should only used when you have to work with exceptions. When there is no special reason, exceptions should be avoided - they are not here to control the control flow but to tell us that an error occurred which normally can not be handled. They are some runtime thing - and why to use runtime things in a statically typed language? When you use monads the compiler always enforces you to write correct code:

scala> def countLen(xs: List[String]) = Option(xs) collect { case List(str) => str } map (_.length)
countLen: (xs: List[String])Option[Int]

scala> countLen(List("hello"))
res8: Option[Int] = Some(5)

scala> countLen(Nil)
res9: Option[Int] = None

scala> countLen(null) // please, never use null in Scala
res13: Option[Int] = None

With collect you ensure that you don't throw an exception during matching the contents. After that with map one can operate on the String and doesn't care anymore if an empty or a full list is passed to countLen.

Now, look at the documentation of Option, there are more useful methods which allow safe error handling. The only thing to keep in mind is not to use method get or a pattern match on the monad during calculation:

scala> Some(3).get
res10: Int = 3

scala> None.get // this throws an exception
java.util.NoSuchElementException: None.get

// this looks ugly and does not safe you from anything because it is equal
// to a null check
anyOption match {
  case Some(e) => // no error
  case None => // error
}

Now you may ask how to get the contents of a monad? That is a good question and the answer is: you won't. When you use a monad you say something like: Hey, I'm not interested if my code threw an error or if all worked fine. I want that my code works up to end and then I will look what's happened. Pattern matching on the content of a monad (means: accessing their content explicitly) is the last thing which should be done and only if there is no other way any more to go further with control flow.

There a lot of monads available in Scala and you have the possibility to easily create your own. Option allows only to get a notification if something happened wrong. If you wanna have the exact error message you can use Try (for exceptions) or Either (for all other things). Because Either is not really a monad (it has a Left- and a RightProjection which are the monads) it is unhandy to use. Thus, if you want to effectively work with error messages or even stack them you should take a look at scalaz.Validation.

EDIT

To address your edit, it seems that you never know the type you can get. This will complicate things a lot since you have to check the type of each element explicitly. I don't think it is possible to do this in a clean way without the use of a library which can do the typechecks for you. I suggest to take a look at some Scalaz or Shapeless code.

Nevertheless is is far easier to do such type checks in the parser and not in the AST traverser. Thus, I suggest using a JSON parser which can handle a type format given by the user and returns an error if it finds some unexpected content. I don't know if Play! can parse things like the following (which can be done in lift-json):

val json = """{ "foo": [1, 2, 3], "bar": [2, 5] }"""
class Content(foo: List[Double], bar: List[Double])
parse(json).extract[Content]


D&D dice rolling app This JavaScript object is part of a mobile application that rolls dice for our D&D games.

Here are some example of possible inputs from the user:

2d4
2d4 + 3d6
2d4 + 3d6 + 12

function DiceRoller() {
    this.rollDices = function(input){

        var parsedInput = input.split("+");
        var totalScore = 0;

        for (var diceIndex = 0; diceIndex < parsedInput.length; diceIndex++) {

            var diceRoll = parsedInput[diceIndex];

            //If we can't find 'd', it means we only have to add the input
            if (diceRoll.indexOf("d") > 0) {
                var diceParts = diceRoll.split('d');

                //We need a random roll for each dice. ex: 4d6, needs 4 rolls.
                for (var numberOfDiceIndex = 0; numberOfDiceIndex < diceParts[0];numberOfDiceIndex++) {
                    totalScore += getRandomInt(1,diceParts[1]);
                }
            } else {
                totalScore += parseInt(diceRoll);
            }
        }

        return totalScore;
    };

    var getRandomInt = function(min, max){
        return Math.floor(Math.random()*(max-min+1)+min);
    }
}

I'm concerned about the performance of the rollDices method, considering I use split quite a lot and I have nested loops.

Performance here won't be a problem. The splits you have are relatively trivial, and presumably there is a human component in here too (someone entering the roll-specs), so the parse time will essentially be negligible compared to that.

The code itself though has some neat features, and also some concerns.

Random

First up, the getRandomInt method is good. It follows the classic definition for retrieving a random value from a specified closed range [low,high]. I would prefer for there to be spaces on the operators though. Your code:

return Math.floor(Math.random()*(max-min+1)+min);

should be:

return Math.floor(Math.random() * (max - min + 1) + min);

Numbers

Your code makes some lucky guesses about numbers, and although it works, it is also easy to break. For example, if I take your random function again:

return Math.floor(Math.random()*(max-min+1)+min);

and instead make it:

return Math.floor(Math.random()*(1+max-min)+min);

it should work fine, right? Except, it won't, because you give the max value to the function as string:

var diceParts = diceRoll.split('d');

.....
    totalScore += getRandomInt(1,diceParts[1]);
.....

So, since you pass in diceParts[1] as a string, the 1+max-min will become 1 + "4" - 1 which in turn becomes 14 - 1 instead of 5 - 1.... so, your code works, but is it an accident? Well, not really, because you figured this out in the other side of the situation - where there is no d in the spec. For that, you have:

totalScore += parseInt(diceRoll);

You should be more defensive about number-handling in your code. You will run into mysterious bugs at some time in the future if you don't control them now.

parseInt(...)

You should always give parseInt a radix to work with. The documentation says:

  Specify 10 for the decimal numeral system commonly used by humans. Always specify this parameter to eliminate reader confusion and to guarantee predictable behavior. Different implementations produce different results when a radix is not specified.

(note: the bold emphasis is from the documentation, it is not my addition)

Since parseInt() ignores leading whitespace, and stops parsing on the first non-digit, it is OK to give parseInt(...) padded values - you don't need to trim them first.

Map->Reduce

I would have liked to have seen a map-reduce process happening here:

you have a + separated sequence of inputs
each part has a roll/limit
each result should be summed.

I would be tempted to build the process as:

this.rollDices = function(spec) {
    return spec.split(/\+/)
          .map(parseRoll)
          .reduce(function(a,b){return a + b;});
};

The above code describes the process quite well:

split on the +
convert parts to values
sum each part.

The implementation details for parseRoll is:

var getRandomInt = function(min, max){
    return Math.floor(Math.random() * (max - min + 1) + min);
};

var parseValue = function(val) {
    return parseInt(val || "1", 10);
}

var parseRoll = function(roll) {
    var parts = roll.split(/d/);
    var sum = 0;
    var limit = parseValue(parts[1]);
    for (var i = parseValue(parts[0]) - 1; i >= 0; i--) {
        var got = getRandomInt(1, limit);
        sum += got;
        // console.log("From roll " + roll + " part " + i + " got " + got + " sum " + sum);
    }
    return sum;
};

I have put the above in to a snippet here:

var getRandomInt = function(min, max){
    return Math.floor(Math.random() * (max - min + 1) + min);
};

var parseValue = function(val) {
    return parseInt(val || "1");
}

var parseRoll = function(roll) {

    // only here for the snippet
    debug("Parsing " + roll);

    var parts = roll.split(/d/);
    var sum = 0;
    var limit = parseValue(parts[1]);
    for (var i = parseValue(parts[0]) - 1; i >= 0; i--) {
        var got = getRandomInt(1, limit);
        sum += got;

        // only here for the snippet
        debug("  From roll " + roll + " part " + i + " got " + got + " sum " + sum);

    }
    return sum;
};

var rollDices = function(spec) {
    // only here for the snippet
    debugarea.value = "";

    return spec.replace(/[^+0-9d]+/g, "")
          .split(/\+/)
          .map(parseRoll)
          .reduce(function(a,b){return a + b;});
};

// *************************
// only here for the snippet
// *************************

var results = document.getElementById("result");

var debugarea = document.getElementById("debug");

var debug = function(txt) {
    debugarea.value = debugarea.value + txt + "\n";
}

function updated(input) {
    results.value = rollDices(input.value);
}
#spec {
  background-color: lightgreen;
}

#result, #debug: {
  background-color: lightgray;
}
Dice Spec :

<input id="spec" type="text" size="20"  oninput="updated(this)" />

Dice Value:

<input id="result" type="text" size="5" readonly="true">

Debug:

<textarea id="debug" rows="20" cols="40"></textarea>


Phonetic word analyzer class I have a functional class WordAnalyzer that uses espeak to grab the IPA phonetics of a word & uses that information to find rhymes & print the number of syllables:

from difflib import SequenceMatcher
from subprocess import check_output

class AnalyzeWord:
    """
    Uses espeak's output to get a word's phonetics & syllables.
    """
    def __init__(self, arg):
        super(AnalyzeWord, self).__init__()
        self.arg = arg
        self.word = arg
        ipa_vowels = ['ɐ', 'i', 'ɪ', 'e', 'ɛ', 'æ', 'a', 'ə', 'ɑ', 'ɒ', 'ɔ', 'ʌ', 'ɚ', 'o', 'ʊ', 'u', 'eɪ', 'aɪ']
        self.phon = list(check_output(['espeak', \
                             '-q', '-v', 'en-us', '--ipa',\
                             self.arg]).decode('UTF-8').strip())
        for i, x in enumerate(self.phon):
            if self.phon[i] == 'ɪ' and self.phon[i-1] in ipa_vowels:
                    self.phon[i-1:i+1] = [''.join(self.phon[i-1:i+1])]
        self.syll = len([l for l in self.phon if l in ipa_vowels])
    def check_rhyme(self, word):
        word = AnalyzeWord(word)
        sm = SequenceMatcher(None, self.phon, word.phon)
        perc = int(sm.ratio() * 100)
        print("Good rhyme - " + str(perc) + "%") \
                if perc > 50 else print("Not a good rhyme - " + str(perc) + "%")

test1 = AnalyzeWord('steed')
test1.check_rhyme("speed")

I think the class itself isn't as 'pythonic' as it ought to be. In particular, I'd love some help cleaning up the self.phon declaration & the following for loop.

EDIT: If there's a better way to store every IPA vowel character / combination I'd love to hear it!

First off, naming, one of the hardest things in Software Development. I would propose a different name than AnalyzeWord, because it is a word-that-has-been-analyzed object and not an analyzer-of-words object. Maybe something like PhoneticWord, EspeakWord or simply Word might better describe this object.

Next, you call super(), even though you don't have any base-class (except object, implicitly, since you are using Python 3.x). So you don't need it at all.

Regarding the better storing of the IPA vowels, I would first make it a global constant (it does not change between instances of the class. If you want to you could also make it a class constant, same difference. Next, I would make it a set, for which membership testing is \$\mathcal{O}(1)\$ instead of \$\mathcal{O}(n)\$ as it is for a list.

In your check_rhyme function, I would add a if not isinstance(word, AnalyzeWord) check around the casting of the other word. This way you could pass in an already analyzed word. I would also make the function return the match percentage and let the caller do the printing, if they want to. In addition, returning a float here, instead of an int gives the caller even more power. If he really wants an int, let them do the casting (or, if you use this feature, together with the printing, often enough, write another function that does just that).

For the printing itself, I would use str.format.

You should add a if __name__ == '__main__': guard to your testing code so it will not be executed when you do e.g. import ip_word_analyzeer in another script.

from difflib import SequenceMatcher
from subprocess import check_output

class Word:
    """
    Uses espeak's output to get a word's phonetics & syllables.
    """
    ipa_vowels = {'ɐ', 'i', 'ɪ', 'e', 'ɛ', 'æ', 'a', 'ə',
              'ɑ', 'ɒ', 'ɔ', 'ʌ', 'ɚ', 'o', 'ʊ', 'u', 'eɪ', 'aɪ'}

    cmd = ['espeak', '-q', '-v', 'en-us', '--ipa']

    def __init__(self, arg):
        self.arg = arg
        self.word = arg

        self.phon = list(check_output(
            Word.cmd + [self.arg]).decode('UTF-8').strip())
        for i, x in enumerate(self.phon):
            if x == 'ɪ' and self.phon[i - 1] in Word.ipa_vowels:
                self.phon[i - 1:i + 1] = [''.join(self.phon[i - 1:i + 1])]
        self.syll = len([l for l in self.phon if l in Word.ipa_vowels])

    def check_rhyme(self, word):
        if not isinstance(word, Word):
            word = Word(word)
        sm = SequenceMatcher(None, self.phon, word.phon)
        return sm.ratio() * 100

def compare_words(word1, word2):
    if not isinstance(word1, Word):
        word1 = Word(word1)

    perc = word1.check_rhyme(word2)
    if perc > 50:
        print("Good rhyme - {}%".format(perc))
    else:
        print("Not a good rhyme - {}%".format(perc))

if __name__ == '__main__':
    test1 = Word('steed')
    compare_words(test1, 'speed')

Maybe with some more thought (and an example containing a 'ɪ' vowel) that loop could be improved as well.

If you want to go slightly further, you could make this Word class inherit from str, giving it a __str__, __add__, __len__ and so on function. Or you could implement sensible things for that, maybe like so (here I actually made a new class inheriting from Word, so I don't have to re-implement the constructor and such.:

class FancyWord(Word, str):

    def __repr__(self):
        return "".join(self.phon)

    def __str__(self):
        return self.word

    def __add__(self, other):
        return FancyWord(str(self) + str(other))

    def __len__(self):
        return self.syll

    def __iter__(self):
        return iter(self.phon)

With this you can now do these things:

>>> x = FancyWord("speed")
>>> print(x)
speed
>>> x
spˈiːd
>>> repr(x)
spˈiːd
>>> len(x)
1
>>> for phoneme in x: print(phoneme)
s
p
ˈ
i
ː
d

>>> y = FancyWord("steed")
>>> x + y
spˈiːdstiːd

>>> " ".join([x, y])
'speed steed'

The last one works, because of multiple inheritance magic, so basically the second base class str of FancyWord her. Word inheriting from str, as I suggested bove and then implementing these methods would also work.

This might help you with whatever you want to further do with this class.


Including functions in _ViewStart.cshtml I've got the following code in my _ViewStart.cshtml file:

@using System.Text;
@{
    string controllerName = (string)ViewContext.RouteData.Values["controller"];
    ViewContext.Controller.ViewBag.BodyCssClass = ConvertControllerNameToCssClass(controllerName);
}

@functions {
    private static string ConvertControllerNameToCssClass(string text)
    {
        if (text == null)
        {
            throw new ArgumentNullException("text");
        }

        StringBuilder sb = new StringBuilder();
        sb.Append(text[0]);
        for (int i = 1; i < text.Length; i++)
        {
            if (char.IsUpper(text[i]) && text[i - 1] != '-')
            {
                sb.Append('-');
            }
            sb.Append(text[i]);
        }
        return sb.ToString().ToLower();
    }
}

In my layout the ViewBag.BodyCssClass is used like so:

<body class="@ViewBag.BodyCssClass" itemscope itemtype="https://schema.org/WebPage">

Allowing me to target a particular page with CSS specific to that controller where required.

The ConvertControllerNameToCssClass function has limited use, in that it is not likely to be used elsewhere and shouldn't be used elsewhere either.

However, it is my understanding that the _ViewStart.cshtml is supposed to be used for adding code that would otherwise be duplicated across all views. Is it acceptable to add functions to the _ViewStart.cshtml?

Does anyone have any thoughts on this or any alternatives to how they would implement something like this?

The ConvertControllerNameToCssClass function has limited use, in that it is not likely to be used elsewhere and shouldn't be used elsewhere either.

It looks like the function converts a PascalCase input text to dash-separated-words format.
This is actually a quite common text transformation to do.
In some frameworks it's called "slugify",
and I would prefer this shorter name instead of the current very long name.
(Another naming idea: ToCssClass)

You seem to have a bug:
you check if the input is null,
but you don't check if it's empty.
If it's empty, then this line will throw an exception:

    sb.Append(text[0]);

Instead of transforming the input character by character,
it would be much simpler to use a regex.
I don't know the syntax in C#,
I hope this Perl/Bash example might help:

$ perl -pe 's/[A-Z]/-\L$&/g; s/^-//' <<< HelloWorldThere
hello-world-there


Filter a list of persons by age and write the results to a relevant file Let's say I have a class:

class Data
{
    public string Name { get; set; }
    public string LastName { get; set; }
    public int Age { get; set; }

    public Data(string name, string lastname, int age)
    {
        Name = name;
        LastName = name;
        Age = age;
    }
}

I want to get the age of a person, if it's <18 then print all the data to Kids.csv, else I want it to be printed to Adults.csv.

I know it's simple and I managed to do that, but the teacher said that my code didn't look like objective programming. He told me that I need to use method to return array and create other method for writing the array to file or something.

static void Main(string[] args)
    {
        People[] people = new People[MaxPeople];
        people = ReadData();

        WriteKids(people);
        WriteAdults(people);
    }

static void WriteKids(Data[] people)
{
File.WriteAllText(@"Kids.csv", String.Empty);
using (StreamWriter writer = new StreamWriter(@"Kids.csv", true, Encoding.Default))
{
    for (int i = 0; i < Count; i++)
    {
        if (people[i].Age < 18)
        {
            writer.WriteLine("{0};{1};{2}", people[i].Name,              people[i].LastName, people[i].Age);
        }
    }
}

static void WriteAdults(Data[] people)
{
        File.WriteAllText(@"Adults.csv", String.Empty);
        using (StreamWriter writer = new StreamWriter(@"Adults.csv", true, Encoding.Default))
        {
            for (int i = 0; i < Count; i++)
            {
                if (people[i].Age => 18)
                {
                    writer.WriteLine("{0};{1};{2}", people[i].Name, people[i].LastName, people[i].Age);
                }
            }
        }
 }

Why is your class called Data? It's obviously a Person.
In compound words like lastname each "part" needs to be capitalized, so it should be lastName. You did this correctly with LastName, so be consistent.
What is People[]? A collection of Peoples? No, it's a collection of Persons. Also, you didn't provide us with the code for People, not the code for ReadData();.
Why define something -- People[] people = new People[MaxPeople]; -- and then assign it a value on the next line -- people = ReadData();? Combine the two lines.
Why do you use for (int i = 0; i < Count; i++) and not foreach?

Why do File.WriteAllText(@"Kids.csv", String.Empty); and File.WriteAllText(@"Adults.csv", String.Empty);? StreamWriter will create the file if it doesn't exist

Do you want to clear the file in case it exists? Then use false instead of true for the bool append parameter of your StreamWriter constructor.

Matter of fact, considering you're using the default encoding and considering you're overwriting the existing contents if the file exists, why not use a simpler constructor?

WriteKids and WriteAdults do basically the same thing, except for the logic in the if and the filename. So they're candidates to be replaced by a  single method that accepts these as parameter.

Of course, the if logic is hard to represent as a parameter, so that's an indication you've done something wrong. What if the if logic was simply a bool, isAnAdult? That would make it easier.

So the question is: how can you compare something against that bool? The solution: add a (read-only) IsAnAdult property to the Person class, which returns the result of Age > 18.

Of course, I'm again combining things when your teacher told you:

  He told me that I need to use method to return array and create other
  method for writing the array to file or something.

So you should have:

one method that accepts Person[] and a bool isAnAdult as parameters and returns Person[] for which isAnAdult is true or false,
a second method that accepts this filtered Person[] and a filename and creates the file.


Check if an element is present within a linked list - follow up This is a follow up question from here. I have revised my code as
  per the suggestions of peer reviewers. I still feel there is lot of
  scope for improvement in this code.

class Node(object):
    def __init__(self,data, next=None):
        self.data = data
        self.next = next

class LinkedList(object):

    def __init__(self):
        self.head = None
        self.size =0

    def extend(self, seq = None):
        """extends list with the given sequence"""

        for i in range(0, len(seq)):
            node = Node(seq[i])
            self.size +=1
            node.next = self.head
            self.head = node

    def append(self, item):
        """append item to the end of list"""
        node = Node(item)
        node.next = self.head
        self.head = node
        self.size += 1

    def printdata(self):
        """print elements of linked list"""
        node = self.head
        while node:
            print node.data
            node = node.next

    def __iter__(self):
        node = self.head
        while node:
            yield node.data
            node = node.next

    def __contains__(self, item):
        """checks whether given item in list"""
        node = self.head
        while node:
            if node.data == item:
                return True
            node = node.next

    def len(self):
        """returns the length of list"""
        return self.size

    def remove(self, item):
        """removes item from list"""
        node = self.head
        current = self.head.next
        if node.data == item:
            self.size -= 1
            self.head = current
            current = current.next
        while current:
            if current.data == item:
                current = current.next
                node.next = current
                self.size -= 1
            node = current
            current = current.next

    def __str__(self):
        return  str(self.data) + str(self.size)

test cases :

if __name__ == "__main__":

    llist = LinkedList()

    llist.extend([98,52,45,19,37,22,1,66,943,415,21,785,12,698,26,36,18,
    97,0,63,25,85,24])

    print "Length of linked list is ", llist.len()

    llist.append(222)

    print "Length of linked list is ", llist.len()

    llist.remove(22)

    print "Elements of linked list  \n", llist.printdata()
    print "Length of linked list is ", llist.len()

   ## Search for an element in list
    while True:
        item = int(raw_input("Enter a number to search for: "))
        if item in llist:
            print "It's in there!"
        else:
            print "Sorry, don't have that one"

The construct

for i in range(0, len(seq)):
    node = Node(seq[i])

is usually frowned upon. Consider a Pythonic

for item in seq:
    node = Node(item)

I see no reason to default seq to None.
append does not append. It prepends.
remove has an unpleasant code duplication. To avoid special casing the head, consider using a dummy node:

def remove(self, item):
    dummy = Node(None, self.head)
    prev = dummy
    while prev.next:
        if prev.next.data == item:
            prev.next = prev.next.next
            size -= 1
        prev = prev.next
    self.head = dummy.next

printdata may (or shall?) use the iterator:

def printdata(self):
    for node in self:
        print node.data


A faster way to compute the largest prime factor I am self-learning js and came across this problem(#3) from the Euler Project

  The prime factors of 13195 are 5, 7, 13 and 29.

  What is the largest prime factor of the number 600851475143 ?

Logic:

Have an array primes to store all the prime numbers less than number
Loop through the odd numbers only below number to check for primes using i
Check if i is divisible by any of the elements already in primes.

If yes, isPrime = false and break the for loop for j by j=primesLength
If not, isPrime = true

If isPrime == true then add i to the array primes and check if number%i == 0

If number%i == 0% update the value of factor as factor = i

Return factor after looping through all the numbers below number

My code:

function problem3(number){
 let factor = 1;
 let primes = [2]; //array to store prime numbers

 for(let i=3; i<number; i=i+2){  //Increment i by 2 to loop through only odd numbers
  let isPrime = true;
  let primesLength= primes.length;

  for(let j=0; j< primesLength; j++){
   if(i%primes[j]==0){
    isPrime = false;
    j=primesLength; //to break the for loop
   }
  }

  if(isPrime == true){
   primes.push(i);
   if(number%i == 0){
    factor = i;
   }
  }
 }
 return  factor;
}

console.log(problem3(600851475143));

It is working perfectly for small numbers, but is quite very slow for 600851475143. What should I change in this code to make the computation faster?

Edit: Updated code based on feedback

There are many questions about Project Euler 3 on this site already.  The trick is to pick an algorithm that…

Reduces n whenever you find a factor, so that you don't need to consider factors anywhere near as large as 600851475143
Only finds prime factors, and never composite factors, so that you never need to explicitly test for primality.

Your algorithm suffers on both criteria: the outer for loop goes all the way up to 600851475143 (which is ridiculous, because even if you optimistically assume that it takes one nanosecond per loop, that would be 5 minutes), and you're testing each of those numbers for primality (which is incredibly computationally expensive).


Organizing interfaces for player movement in chess game I'm trying to figure out how to organize interfaces in my game to make future development easier, by making my code more modular.

Notice that my Rook class implements the IPlayer interface, which will move it left and right, but not up and down (I had to add that in seperate methods). My question is, should I make an interface per method of movement that I want to do (IMoveLeft, IMoveRight, IMoveUp, IMoveDown)? This makes composing future players more modular, perhaps there is a better way.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Sandbox
{
    class Program
    {
        static void Main(string[] args)
        {
            IPlayer bob = new Person();
            IPlayer rook = new Rook();
            List<IPlayer> iPlayerList = new List<IPlayer>();
            iPlayerList.Add(bob);
            iPlayerList.Add(rook);

            foreach (var player in iPlayerList)
            {
                player.MoveLeft(1);
                player.MoveRight(1);
            }
        }
    }

    public class Person : IPlayer
    {
        public int XDistanceFromOrigin { get; set; }

        public void MoveLeft(int distanceToMove)
        {
            XDistanceFromOrigin = XDistanceFromOrigin - 1;
        }

        public void MoveRight(int distanceToMove)
        {
            XDistanceFromOrigin = XDistanceFromOrigin + 1;
        }
    }

    interface IPlayer
    {
        void MoveLeft(int distanceToMove);
        void MoveRight(int distanceToMove);
    }

    public class Rook : IPlayer
    {
        public int XDistance { get; set; }
        public int YDistance { get; set; }

        public void MoveLeft(int distanceToMove)
        {
            XDistance = XDistance - 1;
        }

        public void MoveRight(int distanceToMove)
        {
            XDistance = XDistance + 1;
        }

        public void MoveUp(int distanceToMove)
        {
            YDistance = YDistance + 1;
        }

        public void MoveDown(int distanceToMove)
        {
            YDistance = YDistance - 1;
        }
    }
}

A player doesn't move left or right, they move a game piece
A rook is not a player. A player has a rook (2 actually, but I digress).

When thinking about interfaces/class hierarchies it's useful to think in terms of "is a" or "has a" relationships. Bob is a player. A rook is a piece. A player has a rook.

I would modify your interfaces a bit.

interface IPlayer
{
    string Name { get; }
    bool MovePiece(IPiece piece, int row, int rank);
}

interface IPiece
{
    int Row { get; set; }
    int Rank { get; set; }

    bool CanMoveTo(int row, int rank);
}

You may want to consider using standard chess notation for clarity and cohesion with the problem domain, but using Cartesian coordinates will make the code simpler to write. I added a CanMoveTo method so each piece can store its own logic for checking to see if it's allowed to move to a position that the player attempts to move it to. This design may not be perfect (or even good), but should get you pointed in the right direction.


Captcha Letter Extraction We are developing an automatic text Captcha solver in python 3. The solver has a module that is responsible for extracting letters out of captcha images that contain 4 letters each. We would like to get a review of this module and understand if there are any ways to write it more elegantly.

Here is an example of a captcha image:

Thanks!

import os
import os.path
import cv2
import glob
import imutils

CAPTCHA_IMAGE_FOLDER = "generated_captcha_images"
OUTPUT_FOLDER = "extracted_letter_images"

# Get a list of all the captcha images we need to process
captcha_image_files = glob.glob(os.path.join(CAPTCHA_IMAGE_FOLDER, "*"))
counts = {}

# loop over the image paths
for (i, captcha_image_file) in enumerate(captcha_image_files):
    print("[INFO] processing image {}/{}".format(i + 1, len(captcha_image_files)))

    # Since the filename contains the captcha text (i.e. "2A2X.png" has the text "2A2X"),
    # grab the base filename as the text
    filename = os.path.basename(captcha_image_file)
    captchaCorrectText = os.path.splitext(filename)[0]

    # Load the image and convert it to grayscale
    image = cv2.imread(captcha_image_file)
    gray = cv2.cvtColor(image, cv2.COLOR_BGR2GRAY)

    # Add some extra padding around the image
    gray = cv2.copyMakeBorder(gray, 8, 8, 8, 8, cv2.BORDER_REPLICATE)

    # Threshold the image (convert it to pure black and white)
    Thresh = cv2.Threshold(gray, 0, 255, cv2.THRESH_BINARY_INV | cv2.THRESH_OTSU)[1]

    # find the contours (continuous blobs of pixels) the image
    contours = cv2.findContours(Thresh.copy(), cv2.RETR_EXTERNAL, cv2.CHAIN_APPROX_SIMPLE)

    # Hack for compatibility with different OpenCV versions
    if imutils.is_cv2():
        contours = contours[0]
    else:
        contours = contours[1]

    letter_image_regions = []

    # Now we can loop through each of the four contours and extract the letter
    # inside of each one
    for contour in contours:
        # Get the rectangle that contains the contour
        (x, y, w, h) = cv2.boundingRect(contour)

        # Compare the width and height of the contour to detect letters that
        # are conjoined into one chunk
        if w / h > 1.25:
            # This contour is too wide to be a single letter!
            # Split it in half into two letter regions!
            half_width = int(w / 2)
            letter_image_regions.append((x, y, half_width, h))
            letter_image_regions.append((x + half_width, y, half_width, h))
        else:
            # This is a normal letter by itself
            letter_image_regions.append((x, y, w, h))

    # If we found more or less than 4 letters in the captcha, our letter extraction
    # didn't work correcly. Skip the image instead of saving bad training data!
    if len(letter_image_regions) != 4:
        continue

    # Sort the detected letter images based on the x coordinate to make sure
    # we are processing them from left-to-right so we match the right image
    # with the right letter
    letter_image_regions = sorted(letter_image_regions, key=lambda x: x[0])

    # Save out each letter as a single image
    for letter_bounding_box, letter_text in zip(letter_image_regions, captchaCorrectText):
        # Grab the coordinates of the letter in the image
        x, y, w, h = letter_bounding_box

        # Extract the letter from the original image with a 2-pixel margin around the edge
        letter_image = gray[y - 2:y + h + 2, x - 2:x + w + 2]

        # Get the folder to save the image in
        save_path = os.path.join(OUTPUT_FOLDER, letter_text)

        # if the output directory does not exist, create it
        if not os.path.exists(save_path):
            os.makedirs(save_path)

        # write the letter image to a file
        count = counts.get(letter_text, 1)
        p = os.path.join(save_path, "{}.png".format(str(count).zfill(6)))
        cv2.imwrite(p, letter_image)

        # increment the count for the current key
        counts[letter_text] = count + 1

Without some sample inputs I can only really comment on how the code reads, less on how it runs.

First, this should really be a few functions, I'll indicate below where might be good places to separate the code.
Why import os and os.path? Just import os then call os.path.<something>
You don't need to wrap i, captcha_image_file in brackets when using enumerate(); enumerate() also takes an optional start argument; so, using f-strings as well, you can write:

for count, captcha_image_file in enumerate(captcha_image_files, 1):
    print(f"[INFO] processing image {count}/{len(captcha_image_files)}")

count typically indicates starting from 1, where pos or index would indicate starting from 0. Avoid using short, non-descriptive variables like i.

The "Hack" can be shortened nicely with a ternary operator:

contours = contours[0] if imutils.is_cv2() else contours[1]

I would separate the block immediately following this into a function, which would yield, rather than append to a list. This will be faster, cleaner, and get rid of dummy variables.
I would use itemgetter rather than a lambda function here, entirely a style choice, though.
Contrary to what I said earlier, I actually think using x, y, w, h is fine here as they are commonly understood to be the x and y directions, width and height.
Again, this last block (for letter_bounding_box, letter_text ...) should be its own function.
Really, I think this whole piece of code should be a class, the globals can then be class variables, and you can have three methods. You can then have a main() function which calls these methods and keeps track of which file you're on etc. This main() should sit inside:
if __name__ == '__main__':
Overall comments are good, somewhat excessive in parts but better than too sparse.

The bulk of the improvements would come from separating out your code into readable methods. Functions / methods should generally have one clear purpose. This also has the advantage of making generators easier to use as you can easily yield rather than constantly making lists.


MVC N-Tier Architecture I am working on a new MVC web application, so the project is in early stages. I have created 4 projects.

Let me explain what they do:

Common --> contains all my DTOs, enums helpers, ViewModels etc.

Web --> simple MVC application

Business --> This contains providers which have an interface and implementation for dependency injection:

DAL --> Contains edmx file and DB Entities

DAL is similar to Business project. It has an IUsersDAL and then UsersDAL implementation.

Do I really need the DAL classes to access the database, or can I just access them from the Business class?
Just for an example I am adding a sample code from UsersProvider.cs:
This what my BusinessProvider is doing:
    private readonly IXPTSecurity _security;
    private readonly IUsersDAL _usersDAL;

    public UsersProvider(IXPTSecurity xptSecurity, IUsersDAL usersDAL)
    {
        _security = xptSecurity;
        _usersDAL = usersDAL;
    }

    public UserSessionModel AuthenticateUser(string userName, string password, ValidationContainer validationContainer)
    {
        var userInfo = _usersDAL.GetUserByUserNamePassword(userName, _security.Encrypt(password));

        if (userInfo == null)
        {
            validationContainer.AddMessage(MessageType.Error, "Login Failed. User name or password is incorrect.");
            return null;
        }

}

And then this is what is happening in the _usersDal.GetUserByUserNamePassword():
// So this is this is what UsersDAL is doing
    public User GetUserByUserNamePassword(string userName, string password)
    {
        return _managerEntities.Users.FirstOrDefault(s => s.sUserName == userName && s.sPassword == password);
    }

As you can see, all I am doing in my DAL classes is a simple LINQ type read from the database. If I do this in the Providers it saves me an extra layer.
It would be nice to get someone else opinion on this before I start coding.
By the way I need a separate Common and Business because this project requires another web application and another 3,4 Windows services, which will be later added using the same Business and Common. But I am not sure on the DAL.

I personally think that what you have done here by separating the layers is better than including this all in one layer....

You're following the Single Responsibility Principle - Business Logic and data retrieval results in two different reasons for change and therefore I believe separating them in different classes is following this principle
Your IUsersDAL can be stubbed in unit testing as this is injected into UsersProvider - you can separately test data access and business logic
Reusing GetUserByUserNamePassword in numerous other business provider's means you don't have to write the same query again therefore you're promoting code reuse.  Your UsersDAL is basically a repository pattern for the entity User as this will be used for storing the numerous LINQ queries associated with data retrieval for that entity.  Maybe think object having repositories for aggregate root objects only.
What if you changed your data retrieval for a User entity from a LINQ query to accessing them from a web service - you never know what might change in your application as it grows and you wouldn't want to be directly referencing web services in your business layer, accessing them through a Repository pattern would be a cleaner way of doing it.

I think developers sometimes doubt the extra layer due to the advantages of using an ORM and Linq as like you have done as you can create the data access retrieval within one query.

ORMs like Entity Framework are Unit Of Work and Repository patterns in themselves this is why having an extra layer to contain the query seems to be trivial, however I believe you're merely extending the repository as you're encapsulating data retrieval logic specific for an entity.


Primality test in Python Here is a working Python implementation of primality test. Is there something that I could change in code to achieve a better running time? For more information about algorithm see this post.

from sympy import *
from sympy.abc import x

n=int(input("Enter a number : "))

def xmat(r,n):
    return Matrix([[2*x, -1], [1, 0]])*rem(1,x**r-1)*(1%n)

def smallestr(n):
    if n==1 or n%2==0:
        return 0
    else:
        r=3
        while r<1000000:
            u=n%r
            if u==0 and r<n:
                return 0
            elif not(u==0) and not(u==1) and not(u==r-1):
                return r
            else:
                r=nextprime(r)

def myisprime(n):
    r=smallestr(n)
    if r==0:
        return n==2
    else:
        xp=(xmat(r,n)**n)*Matrix([[x],[1]])
        return trunc(xp[1],n)==(rem(x*(1%n),x**r-1))**n

if myisprime(n):
    print("prime")
else:
    print("composite")

You can run this code here.

EDIT

To clarify things, this Python code represents my attempt to translate the following PARI/GP code which is very fast.

Ooooh boy, a prime number finder with the question, "how do I make this faster." It's like asking a bartender what their favorite drink is: it really depends. However, before we get to performance, let's tackle some of the stylistic considerations in this code.

Wrap the actual executing code in a if __name__ == '__main__' block.
Follow standard naming conventions for functions and variables (not to mention spacing).
The algorithm you're implementing is fairly complicated. Add some brief comments and docstrings to explain what's going on.
Either use descriptive names for your variables or add comments/docstrings explaining what they are.
Name your constants. Why r < 1000000? This would be clearer if we had something like MAX_PRIME_VALUE = 1000000, r < MAX_PRIME_VALUE.

Now in terms of performance. Are there ways to make this faster? Yes, of course. If you had a dictionary of all prime numbers <= 1000000, then you could do a simple lookup and be done. The ultimate trade-off between runtime and storage space is to simply pre-compute all the answers to your question. Also, it depends on what hardware/software you have available (pure no-library python? Only sympy? Numpy? Numba? Cython? CUDA?). So in order to answer this question, you really need to ask what your use case is and what trade-offs you're able to make.

For the sake of argument, let's use your code above as our basic requirements. Let's say we have access to sympy and basic Python, but nothing else. As a baseline, let's consider a trivial prime number finder that use nothing but pure Python. We'll just count up from 3 up to sqrt(n): it's naive, it's dead simple to write, and it's actually reasonably fast just because Python is a decent language (and it even has an O(sqrt(N)) runtime, which is not a bad place to start):

def naive_is_prime(n):
    if n == 2:
        return True
    if n < 2 or n % 2 == 0:
        return False

    # Consolidated code thanks to GZ0's comment
    return all(n % i != 0 for i in range(3, int(n ** 0.5 + 1), 2))

And a couple benchmarks:

%timeit myisprime(11)
1.23 ms ± 26.2 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
%timeit naive_is_prime(11)
1.11 µs ± 47.7 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

Uh oh, that's not a good place to begin, with the naive code giving us a 1000x speedup. Well, maybe that's just because the numbers are so small; maybe the overhead of using sympy will shine when we start considering larger primes. Unfortunately, it slows down rapidly, and waiting for it to compute if tiny numbers are prime takes an unbearable amount of time:

%time myisprime(61)
Wall time: 28.3 s

whereas the naive implementation above continues to be almost instantaneous up until fairly large primes:

%timeit naive_is_prime(2**31 - 1)  # 2147483647
2.7 ms ± 160 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

I think the moral of the story is that "premature optimization is the root of all evil". I fall into this all the time, and even did so while answering this question (I had a more complicated baseline to show off that performed much worse than that trivial code), and it's very tempting to use a fancy package like sympy to accomplish what's not a very fancy problem. Start simple, then look into some common approaches to the problem (like the Sieve approach) and figure out if they actually help you or not.


Time Code with Microsecond Conversion The function below converts a time code into seconds, while maintaining the decimal value (fractions of seconds). The time code fed to this function will always contain microseconds. This currently works, but I'd be interested to see how it could be improved/simplified.

def Second_Conversion(x): #Takes a string such as "2:22:04.996304"
    HMS = x.split(':')
    SF = x.split('.')
    Full_HMS = x[0:7]
    Split_HMS = Full_HMS.split(':')

    Full_Units = int(Split_HMS[0]) * 3600 + int(Split_HMS[1]) * 60 + int(Split_HMS[2])

    Second_Fractions = int(SF[1])
    Converted_Seconds_Fractions = Second_Fractions /1000000
    print(Converted_Seconds_Fractions)

    Final_Time = Full_Units + Converted_Seconds_Fractions
    print(Final_Time)

I don't know whether you wanted to optimize the code for length or speed or "elegance", but here it is as a one-liner:

def Second_Conversion(x):
    return sum(map(lambda i,j: int(i)*j, x.replace('.', ':').split(':'), [3600, 60, 1, 1e-6]))


Getting time since Unix epoch on both Windows NT and *NIX systems The following snippet of C includes implementations for both Windows NT and POSIX-compliant systems to get the time (with microsecond resolution) since the Unix epoch. In the case of Windows NT, the time is registered as 100-nanosecond ticks since the NT epoch (1/1/1601), so it requires a bit of conversion.

I feel like this is the best solution I could've come up with, but it still feels a bit clunky. See if you can help:

#if defined(__unix__) || defined(unix) || defined(__unix) || defined(__CYGWIN__)
#include <sys/time.h>
static struct timeval lnm_current_time;
#elif defined(_WIN32) || defined(__WINDOWS__)
#include <windows.h>
#include <sysinfoapi.h>
static FILETIME lnm_win32_filetime;
#else
#error lognestmonster: Neither Windows NT nor a POSIX-compliant system were detected.\
        Implement your own system time functions or compile on a compliant system.
#endif

uint64_t lnm_getus(void) {
    uint64_t us;
#if defined(__unix__) || defined(unix) || defined(__unix) || defined(__CYGWIN__)
    gettimeofday(&lnm_current_time, NULL);
    us = (lnm_current_time.tv_sec*1000000+lnm_current_time.tv_usec);
#elif defined(_WIN32) || defined(__WINDOWS__)
    // get system time in ticks
    GetSystemTimeAsFileTime(&lnm_win32_filetime);
    // load time from two 32-bit words into one 64-bit integer
    us = lnm_win32_filetime.dwHighDateTime;
    us = us << 32;
    us |= lnm_win32_filetime.dwLowDateTime;
    // convert to microseconds
    us /= 10;
    // convert from time since Windows NT epoch to time since Unix epoch
    us -= 11644473600000000ULL;
#endif
    return us;
}
```

Using static file-scope variables

static struct timeval lnm_current_time;
// ...
static FILETIME lnm_win32_filetime;

makes your code thread-unsafe: Two “simultaneous” invocations of your function from different threads access the same memory. (For other potential drawbacks of static file-scope variables see for example Are file-scope static variables in C as bad as extern global variables? on Software Engineering.)

And there is no reason to use static variables here. With function local variables

uint64_t lnm_getus(void) {
    uint64_t us;
#if defined(__unix__) || defined(unix) || defined(__unix) || defined(__CYGWIN__)
    struct timeval lnm_current_time;
    gettimeofday(&lnm_current_time, NULL);
    // ...
#elif defined(_WIN32) || defined(__WINDOWS__)
    FILETIME lnm_win32_filetime;
    GetSystemTimeAsFileTime(&lnm_win32_filetime);
    // ...
#endif
    return us;
}

the variables are defined exactly where they are needed, which makes the code easier to read and to argue about, and avoids the threading problem.


Project Euler #9: Finding Pythagorean triplets with a given sum Problem statement:
A Pythagorean triplet is a set of three natural numbers, A < B < C, for which, A² + B² = C².
Given a number N, check if there exists any Pythagorean triplet for which A + B + C = N.
public class SpecialPythagoreanTriplet {

public static void main(String[] args) {
    Scanner scan = new Scanner(System.in);
    int t = scan.nextInt();
    int n=0, i=0, j=0, a=0, b=0, c=0;

    while(t!=0) {
        n = scan.nextInt();
        a=0; b=0; c=0;
        for(i=1; i<n; i++)
            for(j=(i+1); j<=n; j++) {
                //These two loops run to take a pair of numbers. j=(i+1) to avoid repetitions in the future.
                if(isSquare((i*i) + (j*j))) //check if a number is a square
                    if(((int)Math.sqrt((i*i)+(j*j)) + i + j) == n) {
                        a = i;
                        b = j;
                        c = (int)Math.sqrt((i*i)+(j*j));
                    }
            }
        if(c==0)
            System.out.println(-1);
        else
            System.out.println(a*b*c);
        t--;
    }
    scan.close();
}

static boolean isSquare(double t) {
    int a = (int)Math.sqrt(t);
    if(a*a == t)
        return true;
    else
        return false;
}
}

I was working on this program in ProjectEuler and Hackerrank consecutively. It ran perfectly for the input of 1000 and got accepted in Project Euler. But it is giving Terminated Due to Timeout error in most of the cases in Hackerrank.
How can I optimize it more to evade this error?

Reorganising the code

In general, it is a good idea to have your logic computing result and your logic handling output/input as independant as possible. This makes things easier to test, easier to understand and thus easier to optimise. In problems based on Project Euler, this is usually a fairly simple task because the problems are described in such a way that they need little or no input and return a number.

In your case, you could write something like:

import java.util.Scanner;

public class SpecialPythagoreanTriplet {

    public static int getProductForPythagoreanTriplerOfPerim(int n)
    {
        int i=0, j=0, a=0, b=0, c=0;
        for(i=1; i<n; i++)
            for(j=(i+1); j<=n; j++) {
                //These two loops run to take a pair of numbers. j=(i+1) to avoid repetitions in the future.
                if(isSquare((i*i) + (j*j))) //check if a number is a square
                    if(((int)Math.sqrt((i*i)+(j*j)) + i + j) == n) {
                        a = i;
                        b = j;
                        c = (int)Math.sqrt((i*i)+(j*j));
                    }
            }
        return (c==0) ? -1 : a*b*c;
    }

    public static void main(String[] args) {
        if (false) // interactive
        {
            Scanner scan = new Scanner(System.in);
            for (int t = scan.nextInt(); t!=0; t--) {
                System.out.println(getProductForPythagoreanTriplerOfPerim(scan.nextInt()));
            }
            scan.close();
        }
        else // hardcoded
        {
            System.out.println(getProductForPythagoreanTriplerOfPerim(25) == -1);
            System.out.println(getProductForPythagoreanTriplerOfPerim(1000) == 31875000);
            System.out.println(getProductForPythagoreanTriplerOfPerim(30000) == 1197129472);
        }
    }

    static boolean isSquare(double t) {
        int a = (int)Math.sqrt(t);
        if(a*a == t)
            return true;
        else
            return false;
    }
}

Improving the code quality

Then, a few things you could do to improve your code quality:
 - remove useless comment
 - put brackets around any block containing more than 1 line of code even if this is not required
 - define variables in the smallest possible scope
 - factorise out expressions written and computed multiple times.

You'd get something like:

public static int getProductForPythagoreanTriplerOfPerim(int n)
{
    int a=0, b=0, c=0;
    for (int i=1; i<n; i++) {
        for (int j=i+1; j<=n; j++) {
            //These two loops run to take a pair of numbers. j=(i+1) to avoid repetitions in the future.
            int squareCand = i*i + j*j;
            if (isSquare(squareCand)) {
                int squareRoot = (int)Math.sqrt(squareCand);
                if (squareRoot + i + j == n) {
                    a = i;
                    b = j;
                    c = squareRoot;
                }
            }
        }
    }
    return (c==0) ? -1 : a*b*c;
}

Optimising the code

Once you've done this. You do not really need the function isSquare anymore. For performance reason, it would even be a good idea to get rid of it in order not to compute square roots multiple times.

public static int getProductForPythagoreanTriplerOfPerim(int n)
{
    int a=0, b=0, c=0;
    for (int i=1; i<n; i++) {
        for (int j=i+1; j<=n; j++) {
            //These two loops run to take a pair of numbers. j=(i+1) to avoid repetitions in the future.
            int squareCand = i*i + j*j;
            int squareRoot = (int)Math.sqrt(squareCand);
            if (squareRoot*squareRoot == squareCand) {
                if (squareRoot + i + j == n) {
                    a = i;
                    b = j;
                    c = squareRoot;
                }
            }
        }
    }
    return (c==0) ? -1 : a*b*c;
}

Then it is clear that the conditions can be rewritten:

           if (squareRoot*squareRoot == squareCand &&
                squareRoot == n - i - j) {
                    a = i;
                    b = j;
                    c = squareRoot;
            }

Then you can use this in order not to compute any squareRoot:

            int squareRootCand = n - i - j;
            int squareCand = i*i + j*j;
            if (squareRootCand*squareRootCand == squareCand) {
                    a = i;
                    b = j;
                    c = squareRootCand;
            }

Also it is even clearer that we want i + j <= n. Because i < j, we also have 2 * i < n.
Thus, the loop boundaries can be rewritten:

    for (int i=1; 2*i < n; i++) {
        for (int j=i+1; i+j<=n; j++) {

I think a more precise analysis of the mathematical property would lead to a smaller search space. In fact, because i corresponds to the smallest side, we could write: 3 * i < nand i + 2j < n.

Warning: from here, it's just me trying to apply random (and simple) math operations with no promise whatsoever that things will get better.

By limiting the number of times sub-expressions are computed, one could write (but I do not find this very beautiful):

    int a=0, b=0, c=0;
    for (int i=1; 3*i < n; i++) {
        int i2 = i*i;
        int rem = n-i;
        for (int j=i+1; 2*j<=rem; j++) {
            int squareRootCand = rem - j;
            int squareCand = i2 + j*j;
            if (squareRootCand*squareRootCand == squareCand) {
                    a = i;
                    b = j;
                    c = squareRootCand;
            }
        }
    }

Then using mathematical property: squareRootCand*squareRootCand = (rem - j)^2 = rem^2 -2*rem*j + j^2 and a few simplifications:

    for (int i=1; 3*i < n; i++) {
        int i2 = i*i;
        int rem = n-i;
        int rem2 = rem*rem;
        for (int j=i+1; 2*j<=rem; j++) {
            if (rem2 - i2 == 2 * rem * j) {
                    a = i;
                    b = j;
                    c = rem - j;
            }
        }
    }

Getting a bit crazy, we can see that we actually want j == (rem2 - i2) / (2*rem) and the division to work fine:

    for (int i=1; 3*i < n; i++) {
        int i2 = i*i;
        int rem = n-i;
        int rem2 = rem*rem;
        int tmp1 = rem2 - i2;
        int tmp2 = 2 * rem;
        if (tmp1 % tmp2 == 0)
        {
            int tmp3 = tmp1 / tmp2;
            for (int j=i+1; 2*j<=rem; j++) {
                if (tmp3 == j) {
                        a = i;
                        b = j;
                        c = rem - j;
                }
            }
        }
    }

and then we do not really need the j loop: the code is now a single loop:

public static int getProductForPythagoreanTriplerOfPerim(int n)
{
    // a + b + c = n
    // 0 < a < b < c < n
    // a^2 + b^2 = c^2
    int a=0, b=0, c=0;
    for (int i=1; 3*i < n; i++) {
        int rem = n-i;
        int tmp1 = rem*rem - i*i;
        int tmp2 = 2 * rem;
        if (tmp1 % tmp2 == 0)
        {
            int j = tmp1 / tmp2;
            if (i+1 <= j && 2*j <= rem) { // not needed ?
                a = i;
                b = j;
                c = rem - j;
            }
        }
    }
    return (c==0) ? -1 : a*b*c;
}

Or, making the math more explicit and adding a check for odd values of n:

public static int getProductForPythagoreanTriplerOfPerim(int n)
{
    // a + b + c = n  =>  c = n - (a + b)
    // a² + b² = c² becomes:
    // a² + b² = (n - (a + b))² = n² + a² + b² + 2ab - 2n(a+b)
    // 2 b (n - a) = n (n - 2a)
    // we must have n = 2m
    // b = n (n-2a) / (2n -2a) = 2m(2m-2a) / (4m - 2a)
    //   = 2m(m-a) / (2m -a )
    //   = n(m-a) / (n-a)
    int a=0, b=0, c=0;
    if (n % 2 == 0) {
        int m = n/2;
        for (int i=1; 3*i < n; i++) {
            int top = n*(m-i);
            int down = n-i;
            if (top % down == 0)
            {
                a = i;
                b = top/down;
                c = down - b;
            }
        }
    }
    return (c==0) ? -1 : a*b*c;
}

Going further

It may not be relevant for this problem (but it may be if you continue on Project Euler problems): a different algorithm can be found using formulas for generating Pythagoran triples.


Program to add two fractions in C The  task was:

  You’re given A/B and C/D. You will have to calculate A/B + C/D and answer it as E/F.

  Input specification
  On the first line you’re given the test case number N. Then there will be N line following. Each line consist of 4 integers with two “/” in the middle. For example: the each line will be “A/B C/D”.

  Output specification
  For every test case, print out the sum of the given two fractions in the format: "numerator/denominator".

My code

#include <stdio.h>

int gcd(int a, int b)
{
    if (a == 0)
        return b;
    return gcd(b%a, a);
}

int main()
{
    int T;
    scanf("%d", &T);

    for (int i=1; i<=T; i++)
    {
        int num1, den1;
        int num2, den2;
        scanf("%d%*c%d %d%*c%d", &num1,&den1,&num2,&den2);

        int num3, den3;
        den3 = gcd(den1,den2);
        den3 = (den1*den2)/den3;
        num3 = (num1)*(den3/den1) + (num2)*(den3/den2);
        printf("Case %d: %d/%d\n", i, num3, den3);
    }

    return 0;
}

Overflow bug

If you use the input:

1
1/100000 2/100000

You get the result:

Case 1: 0/14100

The problem is with this line:

    den3 = (den1*den2)/den3;

Here, all three variables are 100000.  But den1*den2 overflows an integer.  It would be better to write this as:

    den3 = (den1/den3)*den2;

This expression will work as long as the common denominator fits in an int.
 However, it will still overflow when the common denominator is larger than what fits in an int.  For this reason, you might want to use long long variables to avoid the problem.

Simplify the result?

If you use the input:

1
1/4 1/4

the answer the program gives is:

Case 1: 2/4

This is correct, but the answer could also be simplified to 1/2.  This is purely a cosmetic suggestion, since the specification didn't say the answer had to be reduced to its simplest form.


I am learning how to properly use Java parameters and arguments I am learning to use parameters and arguments in Java to make my coding efficient. I am open to suggestion if I've done something wrong with this code of mine.
import java.util.Scanner;

public class paramsAndArgs {

    static String name, feeling;

    public static void main(String[] args) {
        doesSomething(name, feeling);
    }

    private static void doesSomething(String name, String feeling) {
        Scanner scan = new Scanner(System.in);

        System.out.println("What is your name?");
        name = scan.nextLine();
        System.out.println("How are you today?");
        feeling = scan.nextLine();

        System.out.println("Hello, " + name + ". I am feeling " + feeling + " too.");
    }
}
```

Even with this simple example, there are a few things I can comment on.

Class names start with a capital letter.  ParamsAndArgs, not paramsAndArgs.

When parameters are passed to a method, those parameters should have values.  In your code, you're passing the doSomething method two parameters that have no values and aren't used in the doSomething method.  They're set in the doSomething method.

While naming a method doSomething is okay when you're not sure what the method does, you should change the name to something meaningful, like getInput, once you know what the method does. Generally methods are named with a verb, noun combination (get, Input).

Here's your code, reworked to be a more accurate example of parameters and arguments.
import java.util.Scanner;

public class ParmsAndArgs {

    static String name, feeling;

    public static void main(String[] args) {
        Scanner scan = new Scanner(System.in);
        getInput(scan);
        System.out.println("Hello, " + name + ". I am feeling " +
                feeling + " too.");
        scan.close();
    }

    private static void getInput(Scanner scan) {
        System.out.println("What is your name?");
        name = scan.nextLine();
        System.out.println("How are you today?");
        feeling = scan.nextLine();
    }

}


Video Game AI State Change I have here a snippet of code from a simple video game AI enemy.  The basic idea behind the enemy is that he can be in one of a few states.  By default he is in a patrolling state, where he moves around at a slow speed to random points within a circle.  While in this state, he is rolling every X seconds to switch to one of his more interesting states.  Currently, he may either transition into a state where he spins rapidly while patrolling as stated above, but at a much faster move speed. Or he will can instead go into a stationary defensive spin.  Here is the current method which handles this state change:

IEnumerator RollForStateChange()
{
    while (true)
    {
        yield return new WaitForSeconds(RollTickTime);

        if (CanRollForStateChange)
        {
            float spinAttackRoll = Random.Range(0, 100);

            if (spinAttackRoll <= SpinAttackChancePerTick)
            {
                StartCoroutine(SpinAttackTimer());
                continue;
            }

            float spinDefenseRoll = Random.Range(0, 100);

            if(spinDefenseRoll <= SpinDefenceChancePerTick)
            {
                StartCoroutine(DefenseSpinTimer());
                continue;
            }
        }
    }
}

While this sort of dice rolling method works just fine for now, I can't help but feel that it is more likely to choose the spin attack, since that roll goes first.  I can also see this becoming more inefficient if I wanted to add more states for the enemy to transition into.  However, I am struggling to come up with a more elegant way to decide which state the enemy should go into.

You are right that this roll is biased.  Instead of using a fixed range and less-than for your rolls, you should just roll once and use specific ranges to choose which action is taken.  Something like:

int spinAttackChancePerTickMin = 0;
int spinAttackChancePerTickMax = 50;

int spinDefenceChancePerTickMin = 50;
int spinDefenceChancePerTickMax = 100;

int roll = Random.Range(0, 100);

if (roll >= spinAttackChancePerTickMin &&
    roll < spinAttackChancePerTickMax)
{
    StartCoroutine(SpinAttackTimer());
    continue;
}

StartCoroutine(DefenseSpinTimer());


Reducing lock contention for a caching utility, or make it totally lockless but threadsafe My Java program is a module called configProxy which fetches configuration entries from a remote node (say from a Redis instance). When the caller calls the get(configKey) method, this module checks in a local cache (a concurrent hashmap) to see if the setting is already cached.  If not, it fetches the setting from a Redis instance, caches it and then returns.

The implementation of this singleton class:

public class ConfigProxy {

/** This is our one-n-only instance **/
private static ConfigProxy _instance = null;

/** default values for some keys which may not be declared **/
private HashMap<String, String> _defaults = null;

private Jedis jedisConfigConnection;

// hash map for storing the data of config proxy as a cache for each
// String,List and Hash
// to prevent constant hitting to Redis
private ConcurrentHashMap<String, String> _cacheStrings = new ConcurrentHashMap<String, String>();
private ConcurrentHashMap<String, List<String>> _cacheLists = new ConcurrentHashMap<String, List<String>>();
private ConcurrentHashMap<String, HashMap<String, Object>> _cacheHashes = new ConcurrentHashMap<String, HashMap<String, Object>>();

/*
 * Takes host and port as parameters to connect with redis instance
 */
private ConfigProxy(String host, int port) throws ConnectionException {

    try {

        jedisConfigConnection = new Jedis(host, port);
        jedisConfigConnection.ping();
    } catch (Exception e) {
        throw new ConnectionException("Unable to connect to Config Server");
    }

}

public static ConfigProxy getInstance(String host, int port)
        throws ConnectionException {

    if (_instance == null) _instance = new ConfigProxy(host, port);
    return _instance;
}

/**
 * Overloaded version which doesn't take any parameters. Inorder for this
 * call to succeed, the caller, somewhere should have called
 * getInstance(host, port), once. else this throws InitializationException.
 *
 * @return
 * @throws NotInitializedException
 */
public static ConfigProxy getInstance() throws NotInitializedException {
    if (_instance == null) throw new NotInitializedException();
    return _instance;
}

...

/**
 * Returns a configuration setting value either from cache or from a
 * configured Redis instance.
 *
 *  If the given setting was not cached, and it needs to be fetched
 *  from Redis, then the code locks this object
 * @param key
 * @return The value configured for the specified key, if one exists.
 */
public String get(String key) {
    if (_cacheStrings.containsKey(key))
        return _cacheStrings.get(key);
    else {
        synchronized (jedisConfigConnection) {
            String res = (String) jedisConfigConnection.get(key);
            if (res == null) return res;
            _cacheStrings.put(key, res);
            return res;
        }
    }
}

...
...
}

Currently, if the key already exists in the cache, the get(configkey) doesn't lock, and simply returns the value from cache.  However the get function (shown at the bottom in the above code), obtains a lock on jedis connection object, before attempting to fetch data from Redis.

I want to know:

If this is correct, or if I obtain a lock on the entire "this" (configProxy) object.
If it is possible to improve it in any way to further reduce locking contentions, as this will be used in a multi-threaded high txn oriented environment.

EDIT

After review, this is my next version of the get method.

public String get(String key) {

    if (_cacheStrings.containsKey(key)) return _cacheStrings.get(key);
    else {
        String res = null;
        synchronized (jedisConfigConnection) { res = (String) jedisConfigConnection.get(key); }
        if (res != null) _cacheStrings.putIfAbsent(key, res);
        return res;
    }
}

Definitely do not lock on the ConfigProxy instance. Somebody else could acquire a lock on it and then you'd be up the creek. Only lock on private instances that you have the only copy of.
Strictly speaking, you could use putIfAbsent and get rid of the lock altogether, but then you're potentially seeing multiple Redis calls for the same key. You should check both outside and inside the synchronized block to see if the key is there yet. If two threads both hit that barrier in the else, the first one will put a value in the map .. then the second one will go do it again. The first check keeps you out of the synchronized block, and the second one lets you exit it early.
I'm not entirely convinced that a Concurrent map buys you anything here, since you're managing the puts using synchronization. I might be wrong - there may be black magic around gets that I'm unaware of.
Random convention stuff - Java doesn't use underscores to indicate instance variables. Always use curly braces. Make everything final that you can.


Random password generator in C This is a simple password generator. What do you think about it? I am learning C for a while at school and at home. This just has a mix of symbols, lowercase, uppercase and numbers, with a configurable length.

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

int main()
{
    int i = 0;
    int n = 0;
    int randomizer = 0;
    srand((unsigned int)(time(NULL)));
    char numbers [] = "1234567890";
    char letter [] = "abcdefghijklmnoqprstuvwyzx";
    char letterr [] = "ABCDEFGHIJKLMNOQPRSTUYWVZX";
    char symbols [] = "!@#$%^&*(){}[]:<>?,./";
    printf("\nHow long password:");
    scanf("%d", &n);
    char password[n];
    randomizer = rand() % 4;
    for (i=0;i<n;i++)
    {
        if(randomizer == 1)
        {
            password[i] = numbers[rand() % 10];
            randomizer = rand() % 4;
            printf("%c", password[i]);
        }
        else if (randomizer == 2)
        {
            password[i] = symbols[rand() % 26];
            randomizer = rand() % 4;
            printf("%c", password[i]);
        }
        else if (randomizer == 3)
        {
            password[i] = letterr[rand() % 26];
            randomizer = rand() % 4;
            printf("%c", password[i]);
        }
        else
        {
            password[i] = letter[rand() % 21];
            randomizer = rand() % 4;
            printf("%c", password[i]);
        }
    }
    return main();
}

I see some things that I think could help you improve your code.

Decompose your program into functions

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

Check return values for errors

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

Use more whitespace to enhance readability of the code

Instead of crowding things together like this:

for (i=0;i<n;i++)

most people find it more easily readable if you use more space:

for (i=0; i < n; i++)

Eliminate "magic numbers"

Instead of hard-coding the constants 26 and 4 in the code, it would be better to use a #define or const and name them.

Avoid scanf if you can

There are so many well known problems with scanf that you're usually better off avoiding it.

Don't recursively call main

The main() function can be called recursively in C, but it's a bad idea.  You could blow up your stack and there's really no good reason to do that here.  Just use a loop. See https://stackoverflow.com/questions/4238179/calling-main-in-main-in-c for details.

Use a better random number generator

You are currently using

password[i] = numbers[rand() % 10];

There are a number of problems with this approach. This will generate lower numbers more often than higher ones -- it's not a uniform distribution.  Another problem is that the low order bits of the random number generator are not particularly random, so neither is the result.  On my machine, there's a slight but measurable bias toward 0 with that.  See this answer for details, but I'd recommend changing that to the rand_lim in that link and also duplicated below.

Results

Here's an alternative that uses all of these ideas.  It also gets a length from the command line so there's no need for a prompt or scanf.  It eliminates the need for counting characters and uses the better random number generator mentioned above:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <string.h>

int rand_lim(int limit) {
/* return a random number between 0 and limit inclusive.
 */

    int divisor = RAND_MAX/(limit+1);
    int retval;

    do {
        retval = rand() / divisor;
    } while (retval > limit);

    return retval;
}

char picker(const char *charset) {
    return charset[rand_lim(strlen(charset)-1)];
}

int main(int argc, char *argv[])
{
    if (argc != 2) {
        puts("Usage: pwgen len");
        return 1;
    }
    int len = atoi(argv[1]);
    if (len <= 0) {
       puts("Length must be a positive non-zero integer");
       return 2;
    }
    const char* groups[] = {
        "1234567890",  // numbers
        "abcdefghijklmnoqprstuvwyzx",  // lowercase
        "ABCDEFGHIJKLMNOQPRSTUYWVZX",  // uppercase
        "!@#$%^&*(){}[]:<>?,./",    // symbols
    };
    const size_t numGroups = sizeof(groups)/sizeof(groups[0]);
    srand((unsigned int)(time(NULL)));

    // only proceed if we got a number
    for ( ; len; --len) {
        unsigned group = rand_lim(numGroups-1);
        putchar(picker(groups[group]));
    }
    putchar('\n');
}


Parsing Addresses with GeoPanda's GeoDataFrame Long time reader of Code Review with my first question. I'm self-taught and suspect this code can be improved. The project is really important to me and the team and I could really use the help.

I'm using this as part of a larger script run through the terminal which takes a GeoDataFrame and adds the GPS coordinates as a Point per the specification, and then outputs a GDF again. (I then export it as a geojson file.) So far it's running through ~3500 rows in about 2 hours - which is fine - but I don't think I'm using many coding best practices. I'd like to make it as robust as possible because we have some datasets to run through that are +15000 rows. Does anyone have any feedback on this script?

import pandas as pd
import geopandas as gpd
from shapely.geometry import Point
import geopy as gpy
from geopy.geocoders import GoogleV3
from geopy.extra.rate_limiter import RateLimiter
from geopy.exc import GeocoderTimedOut
from geopy.location import Location

def addressParsing(gdf_obj, delayseconds):
    """
    This takes a whole GeoDataFrame and adds the Geo Coords from the standard address
    before returning the udpated Geodataframe
    """
    # Returned class obj if None
    site = Location("0", (0.0, 0.0, 0.0))

    def do_geocode(address):
        try:
            return geocode_with_delay(address)
        except GeocoderTimedOut:
            return geocode_with_delay(address)

    print(f"starting parser: {gdf_obj.shape}, estimated time: {round(delayseconds * gdf_obj.shape[0] / 60, 2)} min")

    # Initiate geocoder
    geolocator = GoogleV3(api_key=g_api_key)

    # Create a geopy rate limiter class:
    geocode_with_delay = RateLimiter(
        geolocator.geocode,
        error_wait_seconds=delayseconds + 20,
        min_delay_seconds=delayseconds,
        swallow_exceptions=True,
        return_value_on_exception= site
        )

    # Apply the geocoder with delay using the rate limiter:
    gdf_obj['temp'] = gdf_obj['Address'].apply(do_geocode)

    # Get point coordinates from the GeoPy location object on each row, drop z vector data:
    gdf_obj["coords"] = gdf_obj['temp'].apply(lambda loc: tuple(loc.point)[:2] if loc else tuple(site.point)[:2])

    # Create shapely point objects to geometry column:
    gdf_obj["geometry"] = gdf_obj["coords"].apply(Point)

    # Drop intermediate columns
    gdf_obj = gdf_obj.drop(["temp", "coords"], axis=1)

    print("FINISHED - conversion successful - check shape")
    return gdf_obj

Nomenclature

Function names should be snake_case per PEP8, i.e. address_parsing.

Type hinting

For function parameters and return values mainly, type hinting will help define your signatures. For example, delayseconds would probably become delay_seconds: float, and gdf_obj: dict if you don't know a lot about the structure of the dictionary. So:

def address_parsing(gdf_obj: dict, delay_seconds: float) -> dict:

Retries:

    try:
        return geocode_with_delay(address)
    except GeocoderTimedOut:
        return geocode_with_delay(address)

Effectively this will ignore up to one GeocoderTimedOut and retry, but if it occurs twice the exception will fall through. A saner way to represent this is:

TRIES = 2
# ...

for retry in range(TRIES):
  try:
    return geocode_with_delay(address)
  except GeocoderTimedOut as e:
    print('Geocoder timed out')
raise e

Temporary variables

The expression

round(delayseconds * gdf_obj.shape[0] / 60, 2)

is complex enough that it should be assigned to a variable. That said, you're better off using Python's actual time-handling features:

from datetime import timedelta
# ...

est_time = timedelta(seconds=delayseconds * gdf_obj.shape[0])
print(f'Estimated time: {est_time}')  # will do pretty formatting


Detailed Last Row Finder For a project I created a more detailed last row finder which looks at more than one column.

Could you provide any feed back on the code below.

Function lColCount(ws As Worksheet, Optional iWhichRow As Integer = 1) As Long
Dim sMaxCol As String
sMaxCol = Cells(iWhichRow, ws.Columns.Count).Address
lColCount = ws.Range(sMaxCol).End(xlToLeft).Column
Function`

Function lRowCount(ws As Worksheet, Optional iWhichCol As Integer = 1) As Long
Dim sMaxRow As String
sMaxRow = Cells(ws.Rows.Count, iWhichCol).Address
lRowCount = ws.Range(sMaxRow).End(xlUp).Row
End Function

Function Col_Letter(lngCol As Long) As String
Dim vArr
vArr = Split(Cells(1, lngCol).Address(True, False), "$")
Col_Letter = vArr(0)
End Function`

Sub findLast()
Dim LC As Long
Dim LR As Long
Dim LCa As String
Dim ws As Worksheet
Dim i As Integer
Dim arr() As Variant
'Assigns ws = sheet1
Set ws = Sheet1
'Sets LC = to the last column number of default row 1
LC = lColCount(ws)
'I counter = 1
i = 1
'Defines array as 1 to the last column number
ReDim arr(1 To LC)
'Loops untill I = Last column counter + 1
Do While i <> LC + 1
'Sets Lr to last row of the looping column i
LR = lRowCount(ws, i)
'Sets last row to arr(I) value loop
arr(i) = LR
'Adds 1 to I ready for next loop
i = i + 1
Loop
'Finds maximum value in array and uses that as last row
LR = Application.WorksheetFunction.Max(arr())
'Use to check the array value in the Immediate window
Dim Res As Variant
For Each Res In arr()
Debug.Print Res
Next Res
'Col_letter function finds the letter of the column number
LCa = Col_Letter(LC)
'copies range A1 sheet1 and all data to sheet2
ws.Range("A1:" & LCa & LR).Copy Sheet2.Range("A1")
End Sub

You end two functions like this -
End Function`

Which might be a typo, but it causes errors.

Readability
Indentation
Indenting your code makes it a lot easier to follow and maintain
Sub findLast2()
    '<snip>
    Dim i As Integer
    Dim arr() As Variant
    Set ws = Sheet1
    LC = lColCount(ws)
    i = 1
    ReDim arr(1 To LC)
    Do While i <> LC + 1
        LR = lRowCount(ws, i)
        arr(i) = LR
        i = i + 1
    Loop
    LR = Application.WorksheetFunction.Max(arr())
    Dim Res As Variant
    For Each Res In arr()
        Debug.Print Res
    Next Res
    '<snip>
End Sub

Comments
"code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.
Naming
Variable names - give your variables meaningful names.
What is LC? lastColumn. LR? lastRow What is vArr() an array of?
Additionally, Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.
Structure
If possible, you should pass argument ByVal rather than ByRef.
You also declare all of your subs and functions as Public by default, so call them public or private to ensure it's clear
Public Sub FindLastRow()
Private Function ReturnColumnLetter(ByVal columnNumber As Long) As String

and so on.


Count sub-sequences of an array having median lying in that subsequence itself We need to calculate the number of sub-sequences of an array having its median lying in the sub-sequence itself. My code is -

#include <bits/stdc++.h>
using namespace std;

#define MOD 1000000007
#define fori0n for(int i=0;i<n;i++)
#define inputLoop for(int j=0;j<t;j++)

// FAST SCANNING ..
template<typename T> void scan(T &x)
{
     x = 0;
     bool neg = 0;
     register T c = getchar();

     if (c == '-')
     neg = 1, c = getchar();

     while ((c < 48) || (c > 57))
     c = getchar();

     for ( ; c < 48||c > 57 ; c = getchar());

     for ( ; c > 47 && c < 58; c = getchar() )
             x= (x << 3) + ( x << 1 ) + ( c & 15 );

      if (neg) x *= -1;
}

// FAST PRINTING..
template<typename T> void print(T n)
{
     bool neg = 0;

     if (n < 0)
     n *= -1, neg = 1;

     char snum[65];
     int i = 0;
     do
     {
          snum[i++] = n % 10 + '0';
          n /= 10;
     }

     while (n);
     --i;

     if (neg)
       putchar('-');

     while (i >= 0)
         putchar(snum[i--]);

     putchar('\n');
}

float median(vector<int> new_array, int num){
       sort(new_array.begin(), new_array.end());
       float median = (num % 2 != 0) ? (new_array[((num+1)/2)-1]) : (float)(new_array[(num/2)-1] + new_array[num/2]) / 2;
      return median;
}

void subsetsUtil(vector<int>& A, vector<vector<int> >& res,
            vector<int>& subset, int index)
{
for (int i = index; i < A.size(); i++) {

    // include the A[i] in subset.
    subset.push_back(A[i]);
    res.push_back(subset);

    // move onto the next element.
    subsetsUtil(A, res, subset, i + 1);

    subset.pop_back();
}

return;
}

vector<vector<int> > subsets(vector<int>& A)
{
    vector<int> subset;
    vector<vector<int> > res;

    // include the null element in the set.
    //res.push_back(subset);

    // keeps track of current element in vector A;
    int index = 0;
    subsetsUtil(A, res, subset, index);

   return res;
}

int main()
{

//ios_base::sync_with_stdio(false);
//cin.tie(NULL);

int t;
scan(t);
//cin>>t;
inputLoop {
int n;
scan(n);
//cin>>n;

// find the subsets of below vector.
vector<int> arr;

int input;
fori0n {
    //cin>>input;
    scan(input);
    arr.push_back(input);
}

vector<vector<int> > res = subsets(arr);
int goodMedian = 0;
// Print result
for (int i = 0; i < res.size(); i++) {

    //cout<<"Sub set : "<<i<<" _ With Size :  "<<res[i].size()<<" == ";

    // if size == 1 or 3
    if(res[i].size() % 2 != 0) {
        // there will always be a good median
        //cout<<"GOOD MEDIAN ";
        goodMedian++;
    }
    else if(res[i].size() == 2) {
        if(median(res[i], 2) == res[i][0] || median(res[i], 2) == res[i][1]) {
            //cout<<"GOOD MEDIAN ";
            goodMedian++;
        }
    }
    else if(res[i].size() % 2 == 0) {
        int size = res[i].size();
        if(median(res[i], res[i].size()) == res[i][size / 2] || median(res[i], res[i].size()) == res[i][(size - 1)/2]) {
            //cout<<"GOOD MEDIAN ";
            goodMedian++;
        }
    }

    //for (int j = 0; j < res[i].size(); j++)
    //  cout << res[i][j] << " ";
    //cout << endl;

}
      print(goodMedian % MOD);
    }
   return 0;
}

Can anyone suggest any better algorithm for this problem ?

Don't #include  <bits/stdc++.h> but explicitly include proper requested headers.
Try to limite uses of using namespace std;, it lead to painfull errors and weird bug.
Don't use preprocessor constants. Instead, use const and constexpr values.
Don't use macro, write directly code or use (inline) (constexpr) function  if you use the same code at much places.
Try to return value instead of taking an input reference.
Avoid old array in favour of std::array.
Don't use register keyword in c++
Don't assign 1 or 0 to a bool, use true and false instead.
getchar return int, why trying to convert to T.
Try to use C++ iostream instead of getchar or putchar
Instead of magical values, use const char (eg 'A') to express chars bound..
Use curly braces for your statement
Don't try to be smart putting many statement on the same line, separated by comma. It's not.
The expression n *= -1 can by simplified in n = -n
Don't use for loops if you don't want Initialization+Condition+Operation. It's ugly.
Take care about indents.


begin and end overloads for istream_iterator I wrote this pretty straight forward begin and end overloads for istream_iterator.

Is this standards compliant? Any problems with usage?

#include <iostream>
#include <iterator>
#include <fstream>

namespace std
{

    template <typename T>
    std::istream_iterator<T> begin(std::istream_iterator<T> iter)
    {
        return iter;
    }

    template <typename T>
    std::istream_iterator<T> end(std::istream_iterator<T>)
    {
        return std::istream_iterator<T>();
    }

}

int main()
{
    for (std::string s : std::istream_iterator<std::string>(std::cin))
    {
        std::cout << s;
    }
}

Not sure that's how I would have written in.
Its not normal to call std::begin() and std::end() on an iterator.

I would have arranged for a some type to represent the stream as a container:

#include <iostream>
#include <iterator>
#include <fstream>

namespace ThorsAnvil
{
    template<typename T>
    struct StreamAsContainer
    {
        std::istream& str;
        StreamAsContainer(std::istream& str) : str(str) {}
    };

    template <typename T>
    std::istream_iterator<T> begin(StreamAsContainer<T>& c)
    {
        return std::istream_iterator<T>(c.str);
    }

    template <typename T>
    std::istream_iterator<T> end(StreamAsContainer<T>&)
    {
        return std::istream_iterator<T>();
    }

}

int main()
{
    namespace TA=ThorsAnvil;
    for (std::string s : TA::StreamAsContainer<std::string>(std::cin))
    {
        std::cout << s;
    }
}


Copy sheet contents faster in Excel I have code that opens a workbook and copies the sheet to the workbook running the code.  I know activating is generally bad for speed but I can't figure out how to speed this up anymore.  The data area is usually "A1:Q45000" but is different all the time.

Sub AutofillOperations()

    Dim answer As Integer, fd As FileDialog, wb1 As Workbook, wb2 As Workbook, FileName As Variant, file As Variant, location As String, filedate As Date, FileMissing As Boolean, filetime As Variant

    Set wb2 = ThisWorkbook

    With Application
        .ScreenUpdating = False
        .EnableEvents = False
        .DisplayAlerts = False
    End With

    location = Range("K7").Value
    file = UCase(Dir(location))

    FileMissing = True

    While (file <> "")

        If InStr(file, UCase("Operation")) > 0 Then
            filedate = DateValue(FileDateTime(location & file))
            If filedate = Date Then
                FileName = location & file
                Set wb1 = Workbooks.Open(FileName)
                wb1.Activate
                Cells.Copy
                wb2.Activate
                Sheet7.Activate
                Cells.Select
                ActiveSheet.Paste
                wb1.Close
                Application.CutCopyMode = False
                FileMissing = False
                GoTo EndWhile
            End If
        End If
            file = UCase(Dir)
            FileMissing = True
    Wend

EndWhile:

    If FileMissing Then
        MsgBox "File could not be found, please use dialog to verify" & vbNewLine & "modified date and select file.", vbInformation, "File not found"
        Set fd = Application.FileDialog(msoFileDialogFilePicker)

        fd.InitialView = msoFileDialogViewList
        fd.Filters.Clear
        fd.Filters.Add "Excel", "*.xls*"
        fd.Filters.Add "Excel", "*.csv"
        fd.FilterIndex = 1
        fd.AllowMultiSelect = False
        fd.InitialFileName = location
        fd.Title = "Select All file or Cancel to use pasted data"

        answer = fd.Show

        If answer = -1 Then
            FileName = fd.SelectedItems(1)
            Set wb1 = Workbooks.Open(FileName)
            wb1.Activate
            Cells.Copy
            wb2.Activate
            Sheet7.Activate
            Cells.Select
            ActiveSheet.Paste
            wb1.Close
            Application.CutCopyMode = False
        End If
    End If

    With Application
        .ScreenUpdating = True
        .EnableEvents = True
        .DisplayAlerts = True
    End With

    Sheet1.Activate
    Sheet2.Activate

    filetime = TimeValue(FileDateTime(location & file))
    Range("D11").Value = filedate
    Range("D12").Value = filetime

End Sub

There are few places you can make this code faster and more efficient.

Most notably, this block (repeated twice):

FileName = location & file
Set wb1 = Workbooks.Open(FileName)
Cells.Copy
wb2.Activate
Sheet7.Activate
Cells.Select
ActiveSheet.Paste
wb1.Close

can be adjusted to this:

Set wb1 = Workbooks.Open(FileName)
Set ws1 = wb1.Worksheets(1) 'make sure to Dim ws1 as Worksheet

With ws1
    'this section will grab the last row and column of actual data on the sheet, instead of copying **every** cell
    'also assumes headers in column A with no data going past left most column in row 1
    Dim lRow as Long, lCol as Long
    lRow = .Range("A" & .Rows.Count).End(xlup).Row
    lCol = .Cells(1,.Columns.Count).End(xlToLeft).Column

    .Range(.Range("A1"),.Cells(lRow,lCol)).Copy wb2.Worksheets("Sheet7").Range("A1") 'change name as needed

    .Close True
End With

I'll try to do a more full review later, but have to leave now.