Adding data to database using asp.net and sql-server I just want to know your suggestions on improving my code on adding data to the database using asp.net and sql-server. Basically, I've created a form that the admin will fill-up and then when he clicked the submit button, the input values will be added to the database.

As of now, this is how i add data. Here is the code of my submit button.

public int ItemType { get; set; }

protected void Button1_Click(object sender, EventArgs e)
{

    try
    {
        int item_brandId = ConnectionClassBrands.GetIdByBrand(itemBrand.Text);
        string item_model = itemModel.Text;
        double item_price = Convert.ToDouble(itemPrice.Text);
        string item_image1 = Session["PicturePath1"].ToString();
        string item_image2 = Session["PicturePath2"].ToString();
        string item_description = itemDescription.Text;
        string item_necktype = itemNeckType.Text;
        string item_body = itemBody.Text;
        string item_fretboard = itemFretboard.Text;
        string item_fret = itemFret.Text;
        string item_bridge = itemBridge.Text;
        string item_neckpickup = itemNeckPickup.Text;
        string item_bridgepickup = itemBridgePickup.Text;
        string item_hardwarecolor = itemHardwareColor.Text;

        if (itemType1.Checked)
        {
            int item_type = ConnectionClassBrands.GetIdByType(itemType1.Text);
            ItemType = item_type;
        }
        else if (itemType2.Checked)
        {
            int item_type = ConnectionClassBrands.GetIdByType(itemType2.Text);
            ItemType = item_type;
        }
            ConnectionClassGuitarItems.AddStringInstrumentItems(ItemType, item_brandId, item_model, item_price, item_image1, item_image2,
            item_description, item_necktype, item_body, item_fretboard, item_fret, item_bridge, item_neckpickup,
            item_bridgepickup, item_hardwarecolor);

            lblResult2.Text = "Upload successful!" + item_image1 + " and " + item_image2;

            ClearTextFields2();
    }
    catch (Exception ex)
    {
        lblResult2.Text = ex.Message;
    }
}

So the data that i want to add is item typeId,brandId,model,price,image1,image2,
description,necktype,body,fretboard,fret,bridge,neckpickup,bridgepickup and hardwarecolor. That is basically the columns in my database. Now, with the input values that the admin made, it will populate to the given variables above. And then all those values will be passed on to ConnectionClassGuitarItems.AddStringInstrumentItems(ItemType,item_brandId etc..).

Inside the ConnectionClassGuitarItems.AddStringInstrumentItems is this code:

public static void AddStringInstrumentItems(int typeId, int brandId, string model, double price, String itemimage1, String itemimage2, string description, string necktype, string body,
    string fretboard, string fret, string bridge, string neckpickup, string bridgepickup, string hardwarecolor)
{
    MusicStoreDBEntities obj = new MusicStoreDBEntities();

    instrumentItem s = new instrumentItem();

    s.typeId = typeId;
    s.brandId = brandId;
    s.model = model;
    s.price = price;
    s.itemimage1 = itemimage1;
    s.itemimage2 = itemimage2;
    s.description = description;
    s.necktype = necktype;
    s.body = body;
    s.fretboard = fretboard;
    s.fret = fret;
    s.bridge = bridge;
    s.neckpickup = neckpickup;
    s.bridgepickup = bridgepickup;
    s.hardwarecolor = hardwarecolor;

    obj.instrumentItems.Add(s);
    obj.SaveChanges();
}

I am using entity framework to add values to the database. That's pretty much my approach on adding data.

I would personally encapsulate your instantiation of MusicStoreDBEntities in a using statement so that the database connection is properly closed and disposed of.

using (MusicStoreDBEntities obj = new MusicStoreDBEntitiesObj())
{
    //code to add record goes here...
}

I think you should also pass in Instrument Item as a single parameter to AddStringInstrumentItems() rather than each individual property of that object so...

public static void AddStringInstrumentItems(instrumentItem item)
{
    using (MusicStoreDBEntities obj = new MusicStoreDBEntitiesObj())
    {
        obj.instrumentItems.Add(item);
        obj.SaveChanges();
    }
}

And in your instrumentItem class you want a constructor to instantiate it.

public instrumentItem(int typeId, int brandId, string model, double price, String itemimage1, String itemimage2, string description, string necktype, string body,
string fretboard, string fret, string bridge, string neckpickup, string bridgepickup, string hardwarecolor)
{
    this.typeId = typeId;
    this.brandId = brandId;
    // so on
    // so forth
}


Filter objects out of an array in JS I have an array of objects arr. One object looks like this:

obj = {
  name: 'name'
  email: 'email'
}

And then I have an array with strings (emails)

var excludedEmails = ['email1','email2',...]

I want to filter the excluded emails out of the arr.

Is there a better/faster way than this in VanillaJS (I am assuming the strings in excludedEmails are all upper case:

var filteredPersons = []
arr.forEach(function(person){
      if(!excludedEmails.includes(person.email.toUpperCase())){
        filteredPersons.push(person);
      }
})

The filter function is what you're looking for. Here's an example given on the documentation page that should demonstrate exactly how it's used:

var words = ["spray", "limit", "elite", "exuberant", "destruction", "present"];

var longWords = words.filter(function(word){
  return word.length > 6;
});

// Filtered array longWords is ["exuberant", "destruction", "present"]

Additionally, the map function can be useful to iterate through arrays to apply a function to each value in the array instead of just filtering out values based on a bool result (in your case, whether the email is in excluded emails). For a basic check to see if the email address is excluded, you'll want filter. If you do any processing on the emails array (say, sending emails), you may want to look into map.


Finding the Least Common Ancestor of a Binary Tree Input a binary tree (the root node) and 2 other nodes that need to be assessed for finding the least common ancestor (LCA)
Code:
a. Finding the nodes first with either breadthFirstSearch or depthFirstSearch
b. If they both exist, find the LCA.
What searching mechanism would be best here? Using breadthFirst for now.

    import java.util.LinkedList;
    import java.util.Queue;
    import java.util.Stack;

    class Treenode {
      public int value;
      public Treenode right;
      public Treenode left;
      public Treenode(int value) {
        this.value = value;
        this.left = null;
        this.right = null;
    }
    }
    public class SearchTree {
     static boolean v1=false,v2=false;
    public static void main(String[] args) {
    Treenode n1 = new Treenode(5);
    Treenode n2 = new Treenode(2);
    Treenode n3 = new Treenode(4);
    Treenode n4 = new Treenode(1);
    Treenode n5 = new Treenode(3);
    n1.left = n2;
    n1.right = n3;
    n2.left = n4;
    n2.right = n5;
    System.out.println("Found BFS? :"+ breadthFirstSearch(n1, 14));

    System.out.println("Found DFS? :"+ depthFirstSearch(n1, 5));
    System.out.println();

    System.out.println("LCA "+" is "+lca(n1,5,50));

    }
    public static boolean breadthFirstSearch(Treenode root, int value){
    if(root==null){
        return false;
    }
    Queue<Treenode> q1 = new LinkedList<Treenode>();
    q1.add(root);
    while(!q1.isEmpty())
    {
        Treenode temp = q1.peek();

        if(temp!=null) {
            q1.remove();
            if (temp.value == value) return true;
            if (temp.left != null) q1.add(temp.left);
            if (temp.right != null) q1.add(temp.right);
        }
    }
    return false;
    }
    public static boolean depthFirstSearch(Treenode root, int value){
    if(root==null){
        return false;
    }
    Stack<Treenode> s1 = new Stack<Treenode>();
    s1.add(root);

    while(!s1.isEmpty()){
       Treenode temp = s1.peek();
        if(temp!=null){
            s1.pop();
            if (temp.value == value) return true;
            if (temp.right != null) s1.push(temp.right);
            if (temp.left != null) s1.push(temp.left);

        }
        }
        return false;
        }

    public static Treenode lcaHelper(Treenode head, int x,int y){

    if(head==null){
        return null;
    }

    if(head.value == x || head.value ==y){
        if (head.value == y){
            v2 = true;
            return head;
        }
        else {
            v1 = true;
            return head;
        }
    }

    Treenode left = lcaHelper(head.left, x, y);
    Treenode right = lcaHelper(head.right,x,y);

    if(left!=null && right!=null){
        return head;
    }
    return (left!=null) ? left:right;
    }

    public static int lca(Treenode head, int h1, int h2) {
    v1 = breadthFirstSearch(head, h1);
    v2 = breadthFirstSearch(head,h2);
    if(v1 && v2){
        Treenode lca = lcaHelper(head,h1,h2);
        return lca.value;
    }
    return -1;
    }
    }

Introduction

I noticed that your demo binary tree is not sorted by values, so I assume that we are dealing with the unsorted version of the data structure.

Default values for reference fields

public Treenode(int value) {
    this.value = value;
    this.left = null;
    this.right = null;
}

In Java, the reference fields are initialized with the value null by default. You can simply write

public Treenode(int value) {
    this.value = value;
}

Actual algorithms

Your BFS and DFS seem like overkilling it. See Summa summarum for an alternative implementation.

Static vs. non-static

static boolean v1=false,v2=false;

Not good. Consider creating the two variables in the actual LCA routine.

Whitespace

Once again:

static boolean v1=false,v2=false;

You should have a single space before and after each binary operator:

static boolean v1 = false, v2 = false;

Hide the object

Since SearchTree is useless as an object, consider declaring a private constructor.

Summa summarum

All in all, I had this in mind:

Treenode.java:

public class Treenode {

    private final int value;
    private Treenode right;
    private Treenode left;

    public Treenode(final int value) {
        this.value = value;
    }

    public int getValue() {
        return this.value;
    }

    public Treenode getLeftChild() {
        return this.left;
    }

    public Treenode getRightChild() {
        return this.right;
    }

    public void setLeftChild(final Treenode child) {
        this.left = child;
    }

    public void setRightChild(final Treenode child) {
        this.right = child;
    }

    @Override
    public int hashCode() {
        return this.value;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            return false;
        }

        if (getClass() != obj.getClass()) {
            return false;
        }

        final Treenode other = (Treenode) obj;

        return this.value == other.value;
    }

    @Override
    public String toString() {
        return "[" + this.value + "]";
    }
}

SearchTree.java:

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

public class SearchTree {

    private SearchTree() {}

    public static Treenode findLeastCommonAncestor(final Treenode root,
                                                   final Treenode node1,
                                                   final Treenode node2) {
        final List<Treenode> list1 = findPath(root, node1);

        if (list1 == null) {
            return null;
        }

        final List<Treenode> list2 = findPath(root, node2);

        if (list2 == null) {
            return null;
        }

        final Set<Treenode> visitedSet = new HashSet<>(list2);

        for (int i = list1.size() - 1; i >= 0; --i) {
            final Treenode node = list1.get(i);

            if (visitedSet.contains(node)) {
                return node;
            }
        }

        return null;
    }

    private static List<Treenode> findPath(final Treenode root,
                                           final Treenode node) {
        final Map<Treenode, Treenode> parentMap = new HashMap<>();
        dfs(root, null, parentMap);

        if (!parentMap.containsKey(node)) {
            return null;
        }

        final List<Treenode> path = new ArrayList<>();
        Treenode current = node;

        while (current != null) {
            path.add(current);
            current = parentMap.get(current);
        }

        Collections.<Treenode>reverse(path);
        return path;
    }

    private static void dfs(final Treenode currentTreenode,
                            final Treenode parentTreenode,
                            final Map<Treenode, Treenode> parents) {
        parents.put(currentTreenode, parentTreenode);

        if (currentTreenode.getLeftChild() != null) {
            dfs(currentTreenode.getLeftChild(), currentTreenode, parents);
        }

        if (currentTreenode.getRightChild() != null) {
            dfs(currentTreenode.getRightChild(), currentTreenode, parents);
        }
    }

    public static void main(String[] args) {
        /*
              n1                   5
             /  \                 / \
           n2    n3    ---->     2   4
          /  \                  / \
         n4  n5                1   3

        */
        Treenode n1 = new Treenode(5);
        Treenode n2 = new Treenode(2);
        Treenode n3 = new Treenode(4);
        Treenode n4 = new Treenode(1);
        Treenode n5 = new Treenode(3);
        Treenode disconnected = new Treenode(6);

        n1.setLeftChild(n2);
        n1.setRightChild(n3);
        n2.setLeftChild(n4);
        n2.setRightChild(n5);

        System.out.println(findLeastCommonAncestor(n1, n4, n3));
        System.out.println(findLeastCommonAncestor(n1, n2, disconnected));
    }
}

Hope that helps.


Tail-Recursion to get a Map of word counts I want to read a file and store the number of occurrences of each word in a Map, using tail recursion. I came up with the following; it seems to work; does it look like it's right?

def countWordsinFile(reader:java.util.Scanner, wordCounts:Map[String,Int]): Map[String,Int] = {
    if (!reader.hasNext()) wordCounts
    else {
        val word = reader.next()
        countWordsinFile(reader, wordCounts + (word -> {wordCounts.getOrElse(word,0)+1}))
    }
}
// the function is called with a reader from a .txt file and wordCounts is an empty Map()

Is this tail-recursive? If not, then how can I make it so?

Yes, this is tail-recursive. Just annotate countWordsInFile with @tailrec and the compiler will check that for you.
Btw: why not name the method countWords? The Scanner is not limited to read from files.


Merging CSV data into database Base functionality:

Reading a CSV file and inserting in database after replacing values with web macro.
Reading values from CSV @ first header information NO,NAME next to that, then read one by one values from CSV and put into web macro context like below:

context.put("1","RAJARAJAN")

Web macro will replace $(NO) ==>1 and $(NAME)==>RAJARAJAN values taken from CSV once the web macro replacement is done.
Add in statement batch.  Once it reaches 1000, execute the batch.

The code is running as per functionality but it takes 4 minutes to parse 50,000 records.  I need a performance improvement or a change in logic.

Note: I use web macro to replace $(NO) in merge query.

Bala.csv

NO?NAME  ==================>Header Information
1?RAJARAJAN
2?ARUN
3?ARUNKUMAR

Connection con=null;
Statement stmt=null;
Connection con1=null;
int counter=0;
 try{
      WebMacro wm = new WM();
      Context context = wm.getContext();
      String strFilePath = "/home/vbalamurugan/3A/Bala.csv";
      String msg="merge into temp2  A using
      (select '$(NO)' NO,'$(NAME)' NAME from dual)B on(A.NO=B.NO)
               when not matched then  insert (NO,NAME)
                  values(B.NO,B.NAME) when matched then
                  update set A.NAME='Attai' where A.NO='$(NO)'";
      String[]rowsAsTokens;
      con=getOracleConnection("localhost","raymedi_hq","raymedi_hq","XE");
      con.setAutoCommit(false);
      stmt=con.createStatement();
      File file = new File(strFilePath);
      Scanner scanner = new Scanner(file);
     try {
         String headerField;
         String header[];
         headerField=scanner.nextLine();
         header=headerField.split("\\?");
         long start=System.currentTimeMillis();
         while(scanner.hasNext()) {
             String scan[]=scanner.nextLine().split("\\?");
             for(int i=0;i<scan.length;i++){
                 context.put(header[i],scan[i]);
             }
       if(context.size()>0){
             String m=replacingWebMacroStatement(msg,wm,context);
             if(counter>1000){
                 stmt.executeBatch();
                 stmt.clearBatch();
                 counter=0;
             }else{
                 stmt.addBatch(m);
                 counter++;
             }

           }
       }
   long b=System.currentTimeMillis()-start;
   System.out.println("=======Total Time Taken"+b);
     }catch(Exception e){
        e.printStackTrace();
    }
  finally {
     scanner.close();
   }
          stmt.executeBatch();
          stmt.clearBatch();
          stmt.close();
    }catch(Exception e){
      e.printStackTrace();
      con.rollback();

  }finally{
      con.commit();

  }

 // Method For replace webmacro with $
  public static String
  replacingWebMacroStatement(String Query, WebMacro wm,
      Context context)   throws Exception {

Template template = new StringTemplate(wm.getBroker(), Query);
template.parse();
String macro_replaced = template.evaluateAsString(context);
return macro_replaced;
}
 // for getting oracle connection
 public static Connection
  getOracleConnection(String IPaddress,
  String username,String  password,String Tns)throws SQLException{

  Connection connection = null;
  try{
  String baseconnectionurl ="jdbc:oracle:thin:@"+IPaddress+":1521:"+Tns;
  String driver = "oracle.jdbc.driver.OracleDriver";
  String user = username;
  String pass = password;
  Class.forName(driver);
  connection=DriverManager.getConnection(baseconnectionurl,user,pass);
  }catch(Exception e){
   e.printStackTrace();
  }
  return connection;
}

It appears that you are reading records from a file, processing them in some way and then using the processed data to insert records into a table. Some of the processing involves getting data out of the database.

It looks like you are creating one insert statement for each row from the source file.

I think you would be more likely to get better performance if you broke the processing up into two parts. First, read the file, process the data and create the final version that is going to go into the database. Then, in a separate operation, load the data into the database.

In the first part of the process, you may be able to find ways to improve the performance by caching query results.

You may be able to write the results of the processing out to a temporary file that can be easily imported using an Oracle tool. This question may be helpful


Related classes around generic types I'm wondering if this code can be simplified. I'm less than thrilled about the fact that I repeat the Draw method in both CatModel and DogModel. For that matter, I'm not real happy that that method is even in the models at all. Trying to move it up to the AnimalModel superclass forces a cast.

EDIT: The "Model" classes should probably have been named "ViewModel" as that's the role they are playing in the actual project.

namespace GenericInterface
{
  public interface IDrawer {
  }

  public interface IDrawer<TModel>: IDrawer
    where TModel: IAnimalModel<TModel>
  {
    void Draw(TModel model);
  }

  public interface IAnimalModel
  {
    void Draw();
  }

  public interface IAnimalModel<TModel>: IAnimalModel
    where TModel: IAnimalModel<TModel>
  {
    IDrawer<TModel> GetDrawer();  // parent interface might like to know that it can get some kind of drawer, but it gets messy with the types.
  }

  public class CatDrawer: IDrawer<CatModel> {
    // eventually, we want to have different types of CatDrawers.
    public void Draw(CatModel model) {
      Console.WriteLine(model.NWhiskers + " whiskers");
    }
  }

  public abstract class AnimalModel {
    public abstract void Draw();
  }

  public abstract class AnimalModel<TModel>: AnimalModel, IAnimalModel<TModel>
    where TModel: class, IAnimalModel<TModel> {
    public abstract IDrawer<TModel> GetDrawer();
  }

  public class CatModel: AnimalModel<CatModel> {
    // The repetition of "CatModel" in the above line is less than ideal. It is driven by the fact that, as best I can tell, there is no way to say that a type parameter should always be equal to the name of the current class.
    public int NWhiskers { get; set; }
    // <summary>Would eventually become IDrawer<CatModel> GetDrawer(DrawerFactory factory) { return factory.GetCatDrawer(this); }
    public override IDrawer<CatModel> GetDrawer() {
      return new CatDrawer();
    }
    // Once we have different CatDrawers, this would become
    // public override void Draw(DrawerFactory factory) { this.GetDrawer(factory).Draw(this); {
    public override void Draw() {
      this.GetDrawer().Draw(this);
    }
  }

  public class DogModel: AnimalModel<DogModel>
  {
    public string Breed { get; set; }
    public override IDrawer<DogModel> GetDrawer() {
      return new DogDrawer();
    }
    public override void Draw() {
      this.GetDrawer().Draw(this);
    }
  }

  public class DogDrawer: IDrawer<DogModel> {
    public void Draw(DogModel model) {
      Console.WriteLine(model.Breed);
    }
  }

  class MainClass {
    public static void Main(string[] args) {
      // Works. Output is:
      // 9 Whiskers
      // collie
      CatModel cat = new CatModel();
      cat.NWhiskers = 9;
      DogModel dog = new DogModel();
      dog.Breed = "collie";
      List<IAnimalModel> animals = new List<IAnimalModel>(){ cat, dog };
      foreach (IAnimalModel animal in animals) {
        animal.Draw();
      }
    }
  }
}

You have the tightest coupling of interfaces I have seen in my life. You are also restricting them in abnormal manners. For example, IDrawer should not need to know about the IAnimalModel. Instead, it should be worried about IDrawableModel, which IAnimalModel can worry about. This way, the coupling is reduced and things are coupled with appropriate interfaces/classes. You want to reduce your coupling to only couple around what needs to depend on what else. Things that are generic like IDrawer shouldn't have to worry about more specific things. They should be, well, for the buzzword: generic.

Think about it as "what happens if I need something else to draw?" If you have another class/interface that has to draw something, and you need to modify IDrawer to count that in, then something is fishy.

You could test this better by developing around Test-Driven Development. You have interfaces for a reason, they are also very good at TDD. It's easy to mock things with them. (So that you can verify that whatever the thing is works. Especially with generics.)

Please do us all a favour and format your code more appropriately. C# does not use Egyptian braces (at least, not by any best-practices). (Java and JAVA do, but this is neither Java nor JAVA.)

namespace GenericInterface
{
    public interface IDrawer
    {
    }

    public interface IDrawer<TModel> : IDrawer
      where TModel : IAnimalModel<TModel>
    {
        void Draw(TModel model);
    }


Print exact value represented by float in [0,1) Motivated by yet another floating point question on stackoverflow, I decided to print a float value \$x\in[0,1)\$, i.e., the fractional part of a float (I might try printing whole floats later). But not the rounded value like print(x) does but the exact represented value. And only using basic operations:
def print_float(x):

    # Convert float x to integer fraction num/den.
    num, den = 0, 1
    while x:
        x *= 2
        num *= 2
        den *= 2
        if x >= 1:
            x -= 1
            num += 1

    # Print num/den in decimal.
    print('0.', end='')
    while num:
        num *= 10
        print(num // den, end='')
        num %= den
    print()

Testing it against a simpler way using string formatting:
from random import random

for _ in range(3):
    x = random()
    print(f'{x}:')
    print(('%.2000f' % x).rstrip('0'))
    print_float(x)
    print()

Output:
0.1659451324370237:
0.16594513243702369020837750213104300200939178466796875
0.16594513243702369020837750213104300200939178466796875

0.6127401513193578:
0.6127401513193577731186678647645749151706695556640625
0.6127401513193577731186678647645749151706695556640625

0.11658146732832175:
0.11658146732832175285210496440413407981395721435546875
0.11658146732832175285210496440413407981395721435546875

Separation of Concerns
Your function is doing too much.  It is:

Converting the float to a fraction
Converting the fraction to a string of digits
Printing the string of digits

You should separate these.
Float to Fraction
First, let's move the float-to-fraction code into its own function.
from typing import Tuple

def float_to_fraction(value: float) -> Tuple[int, int]:
    """
    Convert a floating point number in the range [0, 1) into a fraction.

    >>> float_to_fraction(0.625)
    (5, 8)

    >>> float_to_fraction(0.1)
    (3602879701896397, 36028797018963968)
    """

    if not (0 <= value < 1):
        raise ValueError("Value out of range (0 <= value < 1)")

    numerator, denominator = 0, 1
    while value:
        value *= 2
        numerator *= 2
        denominator *= 2
        if value >= 1:
            value -= 1
            numerator += 1
    return numerator, denominator

Here, I've added:

type hints, describing the input and output types for the function,
a """docstring""" describing how to use the function,

including an example formatted for use with the doctest module,

input range validation, since your expectation is for the range to be within a narrow range of float values.

I've also changed num to numerator, since it would be easy to misinterpret the abbreviation to mean number.  Similarity, den became denominator and x became value.
Fraction to String
Again, converting a numerator/denominator fraction into a series of digits is logical unit of code, which could be reused elsewhere, so I've made it into a function:
def fraction_to_string(numerator: int, denominator: int) -> str:
    """
    Convert a proper fraction into a corresponding series of digits.

    >>> fraction_to_string(5, 8)
    '0.625'
    """

    if not (0 <= numerator < denominator):
        raise ValueError("Improper or negative fraction given")

    if denominator & (denominator - 1) != 0:
        raise ValueError("Denominator must be a power of 2")

    digits = '0.'
    while numerator:
        numerator *= 10
        digits += str(numerator // denominator)
        numerator %= denominator

    return digits

Note: str(0.0) returns '0.0' but your code (and my duplication of it here) returns '0.'.
Your code originally guaranteed the denominator was a power of 2.  With this new function, a caller could ask for fraction_to_digits(1, 3) which would be an infinitely long string of digits, so I've added a condition restricting the denominator to a power of 2.
Float to String
Now we can compose these two functions, as well as recreate the original function's functionality:
def float_to_string(value: float) -> str:
    numerator, denominator = float_to_fraction(value)
    return fraction_to_string(numerator, denominator)

def print_float(x):
    print(float_to_string(x))

if __name__ == '__main__':
    import doctest
    doctest.testmod(verbose=True)

    from random import random

    for _ in range(3):
        x = random()
        print(f'{x}:')
        print(('%.2000f' % x).rstrip('0'))
        print_float(x)
        print()

Additionally, I've added a "main guard" (always a good idea), and I've added a call to doctest.testmod() so the tests embedded in the """docstrings""" are executed.
Notes
The function float_to_fraction(value) may be replaced by the built-in function value.as_integer_ratio().
>>> value = 0.625
>>> value.as_integer_ratio()
(5, 8)


Moving average implementation I've created a MovingAverage class, but it is not optimized for performance at all. I care about performance a little bit because I'm using this class in trading (I collocate on exchange and so on and so on; let's talk about performance of this particular class).

How would you improve it for better performance?

interface Indicator
{
    double? Value { get; }
}

public class Candle
{

    public Candle(double value)
    {
        Minimum = value;
        Maximum = value;
    }

    public void ValueUpdated(double value)
    {
        if (value < Minimum)
        {
            Minimum = value;
        }
        if (value > Maximum)
        {
            Maximum = value;
        }
    }

    public double Maximum { get; private set; }
    public double Minimum { get; private set; }

    public double Median
    {
        get { return (Maximum + Minimum) / 2; }
    }
}

public class MovingAverage : Indicator
{
    private int _length;
    public Queue<Candle> _candles;
    private Candle _newestCandle;
    private double _sumExceptNewest;

    /**
     * only median supported now
     */
    public MovingAverage(int length)
    {
        _length = length;
        _candles = new Queue<Candle>(length);
    }

    public void Add(double val)
    {
        if (_candles.Count == _length)
        {
            _sumExceptNewest -= _candles.Dequeue().Median;
        }
        if (_candles.Count > 0)
        {
            _sumExceptNewest += _newestCandle.Median;
        }
        // TODO: avoid new
        _newestCandle = new Candle(val);
        _candles.Enqueue(_newestCandle);
        RecalculateValue();
    }

    public void Modify(double val)
    {
        if (_newestCandle == null)
        {
            Add(val);
        }
        else
        {
            _newestCandle.ValueUpdated(val);
            RecalculateValue();
        }
    }

    private void RecalculateValue()
    {
        Value = (_sumExceptNewest + _newestCandle.Median)/_candles.Count;
    }

    public double? Value { get; private set; }

    public void CheckValueAndPrintDelta()
    {
        double result = 0;
        foreach (var candle in _candles)
        {
            result += candle.Median;
        }
        result /= _candles.Count;
        Console.WriteLine("checked MA Value = " + Value + " Should be = " + result +
            " delta (1 = 100%) = " + (Value - result) / result);
    }

}

If performance of this code is critical, then it could make sense to avoid heap allocations for Candles. I think the most reasonable way to do that would be make Candle into a struct.

Though mutable value types are evil, so I would also refactor Candle to be immutable. This also means the implementation of _newestCandle would have to change, probably into a pair of double fields (or, alternatively, a separate mutable and resettable class).

I don't see any other potential performance issue in your code. But when it comes to performance, you should always rely on profiling, not your (or someone else's) intuition.

Also, I don't like some names of your methods. Specifically:

ValueUpdated. Method names should usually be in the form “do something”, not “something happened”. So I think a better name would be UpdateValue.
Add, Modify. These are the two fundamental operations of your MovingAverage and I think that those names don't express the meaning well. I would call them something like MoveAndSetCurrent and SetCurrent, respectively. Though such naming indicates that the fundamental operations should rather be Move and SetCurrent.


Move objects into embedded arrays I'm using a document store that stores files and folders in a database and can output the contents (or part of the contents) as a serie of nested objects, like this:

var a = [
  {
    id: 0,
    children: [
      {
        id: 1,
        parent: 0,
        children: [
          {
            id: 2,
            children: []
          }
        ]
      }, {
        id: 3,
        parent: 0,
        children: []
      }, {
        id: 4,
        parent: 0,
        children: [
          {
            id: 5,
            parent: 4,
            children: [
              {
                id: 6,
                parent: 5,
                children: []
              }, {
                id: 7,
                parent: 5,
                children: []
              }, {
                id: 8,
                parent: 5,
                children: []
              }
            ]
          }
        ]
      }
    ]
  }
]

I'm displaying this and allowing users to interact with it in the browser using the HTML5 drag and drop API. When a user drags an element and drops it into another I want to change the object's position in the data. All I have available to use is the object to be moved and the destination object.

I've written a function that handles the move. It works by:

Finding the item to move, the original parent and the new destination.
Removing the item from the parent's children array.
Updating the parent on the item to move.
Pushing the item to the destinations children array.

Here's the code

function findItem (array, id) {
  var result
  array.some(item => {
    if (item.id === id) {
      result = item
      return true
    }
    if (item.children.length) {
      let subResult = findItem(item.children, id)
      if (subResult) {
        result = subResult
      }
    }
  })
  return result
}

function move (itemId, destinationId) {
  let itemToMove, parentItem, destinationItem
  itemToMove = findItem(a, itemId)
  parentItem = findItem(a, itemToMove.parent)
  destinationItem = findItem(a, destinationId)

  // Remove the item from the the parents childen
  var index
  parentItem.children.find((child, idx) => {
    if (child.id === itemToMove.id) {
      index = idx
      return true
    }
  })
  parentItem.children.splice(index, 1)

  // Update the parent of the item to move
  itemToMove.parent = destinationId

  // Push it to the new parent
  destinationItem.children.push(itemToMove)
}

move(3, 1)

Is there a better way of doing this?

Your findItem function looks good as it takes all the input and return an output.

I think I could just hightlight few points here:

The parameters name

Using array and id is not so helpful. In your example is complicated to find a name for the first parameter as the data is very generic. But is not exactly an array, isn't it?

Rather it is a tree and the findItem is a recursive function.

If you use tree and node instead of item the language is more connected to the real data structure.

Of course findItem became findNode.

I'll suggest you to change id with filter just to have a more abstract concept, if you have to change it in something like a function in the future.

usage of array methods

You are using the some method of the array object, and assign a variable to get the result.
Maybe you can solve the problem using just the find method:

function findNode(tree, filter) {
    return tree.find(function (node) {
        return (node.id === filter) || ((node.children.length) && findNode(node.chiltren, filter))
    })
}

As you see the code is more concise and you don't need assignment anymore.

If you consider to use arrow function it will became even more short:

const findNode = (tree, filter) =>
    (tree.find((node) =>
        (node.id === filter) || ((node.children.length) && findNode(node.chiltren, filter))
    ))

And using a function instead of id comparison:

// This returns a function that filter from a specified id
const findById = (searchId) =>
    (node) => (node.id === searchId)

const findNode = (tree, filter) =>
    (tree.find((node) =>
        filter(node) || ((node.children) && node.chiltren.find(filter))
    ))

// And then you use it so:
const node = findNode(tree, findById(1));

About the move function I think you should change the signature and add the tree object as you had done well for the findItem function.

The big advantage of this is on avoiding issues connected to global variable or renaming variables and so on.

So it became like:

function move (nodeId, destinationId, tree)

I changed itemId in coherence with what I wrote before.

If you use let please continue for variables, and please start to use const to constants, I mean variables that don't need to be reassigned.

So don't use var if you don't have to write code that should be portable on old browsers versions.

I know about hoasting but what is the purpose to do that:

...
let itemToMove, parentItem, destinationItem
itemToMove = findItem(a, itemId)
parentItem = findItem(a, itemToMove.parent)
destinationItem = findItem(a, destinationId)
...

You can just write:

  const itemToMove = findItem(a, itemId)
  const parentItem = findItem(a, itemToMove.parent)
  const destinationItem = findItem(a, destinationId)

Applying the suggestion already provided you can rewrite this lines:

  const nodeToMove = findNode(tree, findById(nodeId))
  const parentNode = findNode(tree, findByParent(nodeToMove))
  const destinationNode = findNode(tree, findById(destinationId))

About the search and remove item in the children array, firstly you should avoid changing data structure, and start to create new version instead.

In this particular case you can change your code as follow:

parentNode.chiltren = parentNode.children
    .filter((node) => (node.id !== nodeToMove.id))

That returns a new array without the selected node.


Reading multiple csv files in a single dataframe I have a lot of compressed csv files in a directory. I want to read all those files in a single dataframe. This is what I have done till now:

df = pd.DataFrame(columns=col_names)
for filename in os.listdir(path):
    with gzip.open(path+"/"+filename, 'rb') as f:
        temp = pd.read_csv(f, names=col_names)
    df = df.append(temp)

I have noticed that the above code runs quite fast initially, but it keeps on getting slower and slower as it reads more and more files. How can I improve this?

Ultimate optimization

avoid calling pd.DataFrame.append function within a loop as it'll create a copy of accumulated dataframe on each loop iteration. Apply pandas.concat to concatenate pandas objects at once.

no need to gzip.open as pandas.read_csv already allows on-the-fly decompression of on-disk data.

compression : {‘infer’, ‘gzip’, ‘bz2’, ‘zip’, ‘xz’, None}, default ‘infer’

avoid hardcoding filepathes with path+"/"+filename. Instead use suitable os.path.join feature: os.path.join(dirpath, fname)

The final optimized approach:
import os
import pandas as pd

dirpath = 'path_to_gz_files'   # your directory path
df = pd.concat([pd.read_csv(os.path.join(dirpath, fname))
                for fname in os.listdir(dirpath)], ignore_index=True)


Finding the right structure for my application I have three different types of XML files (structure) which I would like to import into a database.

Every hour a cronjob calls my import.php with a specific XML file type as a parameter.

The import script includes a bootstrap.php (AutoLoader, Setup Doctrine DB Connection) and puts all files matching the name convention of this type in an array and calls a static method for this class.

Simplified code (import.php):

<?php
require_once("bootstrap.php");

function getFiles($pattern)
{
    $fileList = NULL;
    foreach( glob('xml/*'.$pattern.'.xml') as $filename )
    {
        $fileList[] = $filename;
    }

    return $fileList;
}

switch($xmlType)
{
    case "TypeA":
    TypeA::importFeed(getFiles($xmlType));
    break;

case "TypeB":
    TypeB::importFeed(getFiles($xmlType));
    break;

case "TypeC":
    TypeC::importFeed(getFiles($xmlType));
    break;
}

Currently, the classes (TypeA, TypeB, TypeC) contain no other method except importFeed.

Inside importFeed, only 2 things are equal in each class:

Getting the entityManager (Doctrine) by calling $em = app::getEm();
Convert XML into SimpleXMLElement using my parser class $feedObj = Parser::xmlToObject($xmlFile);

Everything else in importFeed differs from class to class because of the different XML types. Only a few tasks repeating e.g. check if an entity already exists in the database.

My first thought was creating a Helper class for this and some other repeating tasks I also need outside the import-script in this application.

But I'm not sure if this helper class should be a normal, "static" or "singleton" class.

The helper-methods doesn't need a specific object instance so static-methods would a good choice. But in some methods I need access to the DB and I do not wont to call app::getEm(); in every method because I don't think this is a good/clean solution.

At this point I challenged my current application design and read a lot of articles about this. The point is the application is not large and it should not be so using a Framework like Symfony is completely exaggerated - nonetheless I want a good solution.

All three classes (TypeA/B/C) have the same function that looks like this:

public static function importFeed($xmlFiles)
{
   $em = \App::getEm();
   foreach($xmlFiles as $xmlFile)
   {
      try
      {
         $feedObj = Parser::xmlToObject($xmlFile);
         ...import xml data to database - differns ...
      }
      catch(\Exception $e)
      {
         ....
      }
   }
}

Only the "..." area differs a lot because of the different XML structure but this part isn't really important for the question. In short: A Doctrine entity is created and many setAttributeX methods are called and in the end $em->flush().

My feeling tells me that it is not right to have three classes with only static methods to import XML files, but I can't find a better solution.

I would go somewhere in the lines of the following code:

abstract class ObjectImporter {

    protected $em;

    public function __construct($em) {
        $this->em = $em;
    }

    abstract public function import($object);
}

class TypeAImporter extends ObjectParser {

    public function __construct($em) {
        parent::__construct($em);
    }

    public function import($object) {
        #do your import magic
    }
}

class XmlImporter {

    private $importer;

    public function __construct($importer) {
        $this->importer = $importer;
    }

    public function import($files) {
        foreach ( $files as $file ) {
            $obj = Parser::xmlToObject($file);
            $this->importer->import($obj);
        }
    }
}

switch ( $xmlType ) {

    case 'TypeA':
        $importer = new XmlImporter(new TypeAImporter(\App::getEm()));
    break;
}

$importer->import($files);

Some notes: the code is not ideal (names aren't that good imo) and I don't like the Parser::xmlToObject method. The Parser should be injected into the XmlImporter class aswel. This way its easier tested and you can also very easily change the xml Parser.

I hope the code is of any use.


Functions taking iterables: peaks, compress, and skipper I am just testing out a few small functions for learning purposes:

def peaks(iterable):
    # returns a list of int for those values in the iterable that are bigger
    # than the value preceding and following them.
    itr = iter(iterable)
    peak =[]
    curr = next(itr)
    last = next(itr)
    try:
        while True:
            first = curr
            curr = last
            last = next(itr)
            if curr > first and curr > last:
                peak.append(curr)
    except:
        pass
    return peak

def compress(v_iterable,b_iterable):
    #takes two iterables as parameters: it produces every value from the first iterable that
    # has its equivalent position in the second iterable representing what Python would consider
    # a True value. Terminate the iteration when either iterable terminates
    mega_it = dict(zip(v_iterable, b_iterable))
    for nxt in sorted(mega_it):
        if mega_it[nxt]:
            yield nxt

def skipper(iterable,n=0):
    #akes one iterable and one int (whose default value is 0) as parameters: it produces values from
    # the iterable, starting at the first one, skipping the number of values specified by the int
    #(skipping 0 means producing every value from the iterable)
    itr = iter(iterable)
    yield next(itr)
    while True:
        for i in range(n):
            skipped = next(itr)
        yield next(itr)

I feel like my codes are lengthy for the kind of work it does. Is there a way to make my functions cleaner or smaller?

Your comments are good, but they should all be written as docstrings instead.

When working with iterators, itertools and generator expressions are your friend.  peaks() works with three iterators, so zip() is useful.

Since the function accepts an iterable, it seems fitting that it should be a generator rather than returning a list.

Python supports double-ended inequalities.

from itertools import tee

def peaks(iterable):
    """Generates values in the iterable that are greater than the value
    preceding and following them."""
    before, this, after = tee(iter(iterable), 3)
    next(this)
    next(after)
    next(after)
    for prev, curr, succ in zip(before, this, after):
        if prev < curr > succ:
            yield curr

If you wanted to keep the old interface, which returns a list, then a list comprehension would be a good way to write it.

def peaks(iterable):
    """Returns a list of values in the iterable that are greater than the
    preceding and following value."""
    xs, ys, zs = tee(iter(iterable), 3)
    next(ys)
    next(zs)
    next(zs)
    return [y for (x, y, z) in zip(xs, ys, zs) if x < y > z]

I find your compress() weird, in that it sorts the results.

You can implement the function using just a generator expression.

def compress(values, incl_excl):
    """Keeping only values whose corresponding entry in incl_excl is true,
    generates the sorted filtered list."""
    yield from sorted(val for val, include in zip(values, incl_excl) if include)

In skipper(), the two occurrences of yield next(itr) should be combined.

def skipper(iterable, n=0):
    """Generator that yields the first element, then skips n values between
    results, until the input is exhausted."""
    iterable = iter(iterable)
    while True:
        yield next(iterable)
        for _ in range(n):
            next(iterable)


string guessing game in java import java.util.Scanner;

class Main {

    public static void main(String[] args) {
        int tries = 0;
        boolean iterated = false;
        String temp = "";
        String holder = "";
        System.out.println("Welcome to the guessing game! Guess one letter at a time. You have 5 incorrect guesses remaining.");

        Scanner reader = new Scanner(System.in);
        System.out.println("The Secret Word Is: ______ (6)");
        String word = "sleepy";

        do {
            System.out.print("Your Guess: ");
            String guess = reader.nextLine();

            for(int i = 0; i < word.length(); i ++) {
                if (guess.equals(Character.toString(word.charAt(i)))) {
                    if(!iterated)
                        temp += Character.toString(word.charAt(i));
                    else {
                        holder = Character.toString(temp.charAt(i)).replace("-", guess);
                        temp = temp.substring(0, i) + holder + temp.substring( i + 1, temp.length());
                    }
                } else {
                    if(!iterated) {
                        temp += "-";
                    }
                }
            }
            tries++;
            iterated = true;
            System.out.println(temp);
            if(temp.equals(word)) {
                System.out.println("You guessed correctly!");
                break;
            }
        }while (tries < 5);

    }
}

So this is the code I have so far. It works pretty well but I was wondering how I could make it so that every time you add enter a letter, it tells you whether it was right or wrong and how many tries you have remaining. Also I was wondering how i could make it so that if the user doesn't guess the word I can say something like "sorry you didn't guess it. the word was (sleepy)". Thank you!

I  reworked your code a little.  Here are the results from one test run.
Welcome to the guessing game! Guess one letter at a time. You have 5 incorrect guesses remaining.
The Secret Word Is: ------ (6)
Your Guess: a
The word does not contain the letter a.
You have 4 incorrect guesses remaining.
------
Your Guess: s
The word contains the letter s.
s-----
Your Guess: d
The word does not contain the letter d.
You have 3 incorrect guesses remaining.
s-----
Your Guess: f
The word does not contain the letter f.
You have 2 incorrect guesses remaining.
s-----
Your Guess: p
The word contains the letter p.
s---p-
Your Guess: g
The word does not contain the letter g.
You have 1 incorrect guesses remaining.
s---p-
Your Guess: h
The word does not contain the letter h.
You have 0 incorrect guesses remaining.
s---p-
Sorry, you didn't guess the secret word "sleepy".

Instead of counting tries up from zero, I counted down from 5.  If you decide to change the number of incorrect guesses, you only have to change it in one place.
I wrote a method to create a dashed word from the word for the secret word message.  I was hoping I could use the dashed word later, but I couldn't.
I still don't understand the character substitution for loop, but it works, so I left it alone.  Using a StringBuilder would greatly simplify your character substitution.
Here's the modified code.
import java.util.Scanner;

public class StringGuessingGame {

    public static void main(String[] args) {
        int incorrectGuesses = 5;
        boolean iterated = false;
        String temp = "";
        System.out.println("Welcome to the guessing game! Guess one letter "
                + "at a time. You have " + incorrectGuesses + " incorrect "
                        + "guesses remaining.");

        Scanner reader = new Scanner(System.in);

        String word = "sleepy";
        String dashedWord = createDashedWord(word);
        System.out.println("The Secret Word Is: " + dashedWord +
                " (" + word.length() + ")");

        do {
            System.out.print("Your Guess: ");
            String guess = reader.nextLine();
            boolean correctGuess = false;

            for (int i = 0; i < word.length(); i++) {
                if (guess.equals(Character.toString(word.charAt(i)))) {
                    correctGuess = true;
                    if (!iterated)
                        temp += Character.toString(word.charAt(i));
                    else {
                        String holder = Character.toString(
                                temp.charAt(i)).replace("-", guess);
                        temp = temp.substring(0, i) + holder +
                                temp.substring(i + 1, temp.length());
                    }
                } else {
                    if (!iterated) {
                        temp += "-";
                    }
                }
            }
            iterated = true;

            if (correctGuess) {
                System.out.println("The word contains the letter " + guess + ".");
            } else {
                System.out.println("The word does not contain the letter " + guess + ".");
                incorrectGuesses--;
                System.out.println("You have " + incorrectGuesses +
                        " incorrect guesses remaining.");
            }

            System.out.println(temp);
            if (temp.equals(word)) {
                System.out.println("You guessed correctly!");
                break;
            }

        } while (incorrectGuesses > 0);

        if (incorrectGuesses <= 0) {
            System.out.println("Sorry, you didn't guess the secret word \"" + word + "\".");
        }

        reader.close();

    }

    private static String createDashedWord(String word) {
        String output = "";
        for (int i = 0; i < word.length(); i++) {
            output += "-";
        }
        return output;
    }

}


Validating user's input before creating graph using TChart The user enters coefficients into input boxes on a form, called GetFunction. Upon clicking an OK button on said form, the input is validated using TryStrToFloat. If all the input boxes are valid,CreateGraphis called. As soon as an invalid input box is found,CreateGraph` should not be called and the user should be able to fix their error.

procedure TfrmGetFunction.btnAddFunctionClick(Sender: TObject);
  var
    i : integer; // Loop counter.
  begin
    for i := 1 to 7 do
      begin
        CoefficientEdit[i].CoValid := CheckCoefficientBoxesValidInput(CoefficientEdit[i].CoEditBox);
        if not CoefficientEdit[i].CoValid then // If any input box is invalid.
          Exit;
      end;
    frmGraphingMode.CreateGraph; // Finally if all input boxes valid.
    frmGetFunction.Visible := false;
    frmGraphingMode.Visible := true;
  end;

Any improvements that I could make? Perhaps removing the Exit may not be a bad idea?

Besides deciding if a coefficient is valid or not the function CheckCoefficientBoxesValidInput can also say what is wrong and suggest how the user can fix it.

In order to communicate such information up to the caller and up to the user interface you can use throwing an exception.

Main success scenario would mean no ifs, just sequence of statements.

Exceptional flow would be encoded in try/except blocks, either in this function or somewhere upwards or in the final Application.OnException handler.

Whether the code would be easier to maintain or not is only opinion-based matter of taste and your established coding conventions.

The main improvement would be that the end-user would know what happened and what to do next.

See also:

normative MSDN: Windows → Dev Center - Desktop → Design → Guidelines → Error Messages
informative MSDN: Documentation → Framework Design Guidelines → Design Guidelines for Exceptions


Function to alternate players' turns, unless a player has won I am doing a coding challenge and it basically says this. There are two players, "black" and "white" and they take turns at a game. If a player won the last turn then it is his turn again, otherwise it is the other player's turn. The whole challenge is here.

Here is the original code I used for that:

function whoseMove(lastPlayer, win) {
  if (lastPlayer == "white") {
      if (win) {
          return "white";
      }
      return "black";
  } else {
      if (win) {
          return "black";
      }
      return "white";
  }
}

however I realised that this could be simplified by changing it to this:

function whoseMove(lastPlayer, win) {
  if (lastPlayer == "white") {
      return win ? "white" : "black";
  } else {
      return win ? "black" : "white";
  }
}

I wonder if this could be simplified even more? Is it possible to have more than one condition in the conditional return statement (I think that is what it is called)?

You could return the same player if win is true, otherwise return the other player, this is done with only 2 tests:

function whoseMove(lastPlayer, win) {
    if (win) return lastPlayer;
    return lastPlayer == "black" ? "white" : "black";
}


Generate a text file representing an email for a job application followup Purpose: After you've submitted a resume and cover letter for a job position, you haven't heard anything back, so you decided to send a follow up email. The script generates the follow up e-mail based on this template.

"""
    followupemail.py

    A script to generate a follow up e-mail after a job application submission.

    Based on this example: https://www.themuse.com/advice/how-to-follow-up-on-a-job-application-an-email-template

"""

def determine_if_string_is_blank(stringtovalidate, parameterType):

    """
        Determine if the stringtovalidate parameter is empty, and if so,
        raise an exception.

        Args:
            stringtovalidate: The string to validate.
            parameterType: What the stringtovalidate parameter represents in your program.

        Returns:
            None. Raises an exception.
    """

    if not stringtovalidate:
        raise ValueError ("{0} is empty", parameterType)

def prompt_user_with_question(question, parameterType):

    """
        Prompt user with a question and call determine_if_string_is_blank to deteremine
        if an answer is empty.

        Args:
            question: the question we're going to ask
            parameterType: What the stringtovalidate parameter represents in your program.

        Returns:
            answer: the string that contains our user answer.

    """

    answer = input(question)
    determine_if_string_is_blank(answer, parameterType)
    return answer

def write_string_to_file(pathtofile,email):

    """
        Writes the email to a .txt file

        Args:
            pathtofile: The path where the e-mail will be stored
            email: The message we're going to write.

        Returns:
            None
    """

    with open(pathtofile, 'w') as f:
        f.write(email)

def main():

    position_title = None
    hiring_manager = None
    company_name = None
    your_name = None

    pathtofile = "email.txt"

    company_name = prompt_user_with_question("Company Name: ", "Company Name").strip()
    position_title = prompt_user_with_question("Position: ", "Position").strip()
    hiring_manager = prompt_user_with_question("Hiring Manager: ", "Hiring Manager")
    your_name = prompt_user_with_question("Your Name: ", "Your Name")

    email = f"Hi {hiring_manager},\n\nI hope all is well. I know how busy you probably are, but I recently applied to the \n" \
         f"{position_title} position, and wanted to check in on your decision timeline.\n\n" \
         f"I am excited about the opportunity to join {company_name} (insert additional details here).\n\n" \
         f"Please let me know if it would be helpful for me to provide " \
         f"any additional information as you move on to the next stage in the hiring process.\n\nI look forward to hearing from you,\n\n{your_name}"

    write_string_to_file(pathtofile,email)

if __name__ == "__main__":
    main()

Explanation: The script prompts the user for company name, position, hiring manager, and the name. It inserts those values into the email string and generates a text file.

You wrote a lot of boilerplate code to do a simple task. Before reading the rest of this review, scroll back up and imagine that you were given the assignment, "Make your code as simple and readable as possible." What would you change about it?

Here's what I'd do.

def determine_if_string_is_blank(stringtovalidate, parameterType):

Inconsistent style. Pick one of snake_case, amoebacase, or camelCase, and stick to it!

This function triggers my "boolean functions should be named with is_ detector. Surely, I think, this function should be named is_blank_string! But in fact your name is (currently) more appropriate, because this function does not return a boolean — it returns None or throws an exception! This is not great design, but, okay, the name isn't the worst. However, I would write this entire function as

def assert_string_isnt_blank(s)
    assert s

(notice that the name of the function now unambiguously tells what it does) and then I would eliminate it.

def prompt_user_with_question(question, parameterType):
    answer = input(question)
    assert answer, f"{parameterType} is empty"
    return answer

By the way, what did you expect this line to do?

raise ValueError ("{0} is empty", parameterType)

Did you perhaps expect it to do the same thing as

raise ValueError ("{0} is empty".format(parameterType))

? It doesn't!

(This suggests that you never tested your code, or at least you never tested its non-success paths. Make sure to test your code before running it in production — or posting it to StackExchange!)

def write_string_to_file(pathtofile,email):

The usual complaint about needing to pick a style (underscores? no underscores?) and sticking to it. But also there's a missing space here. It would be good to get in the habit of running your code through a linter like flake8 before posting it, as well.

$ flake8 --ignore=E501 x.py
x.py:26:25: E211 whitespace before '('
x.py:49:36: E231 missing whitespace after ','
x.py:81:10: E127 continuation line over-indented for visual indent
x.py:86:36: E231 missing whitespace after ','

Nothing major here... but sometimes there is something major, and then you're glad you took the time to look for warnings!

Your docstrings make it look like you're intending this as a 100% complete Python program. If so, then you should start the file with a hashbang:

#!/usr/bin/env python

For the email body itself, consider using """multi-line strings""" instead of backslash-continuations. Multi-line strings can be combined with the f prefix.

position_title = None
hiring_manager = None
company_name = None
your_name = None

You create all these variables and then you never use them again! Or, another way to put it is, you assign None to all these variables and then immediately overwrite it with some other value! Eliminate the dead writes.

In conclusion:

#!/usr/bin/env python

def prompt(question):
    answer = input(question + ": ")
    assert answer, f"{question} is empty"
    return answer

if __name__ == "__main__":
    company_name = prompt("Company Name").strip()
    position_title = prompt("Position").strip()
    hiring_manager = prompt("Hiring Manager")
    your_name = prompt("Your Name")

    with open("email.txt", "w") as f:
        f.write(f"""
Hi {hiring_manager},

I hope all is well. I know how busy you probably are, but I recently applied to the
{position_title} position, and wanted to check in on your decision timeline.

I am excited about the opportunity to join {company_name} (insert additional details here).

Please let me know if it would be helpful for me to provide
any additional information as you move on to the next stage in the hiring process.

I look forward to hearing from you,
{your_name}
        """.strip() + '\n')

Notice that I'm using a function only when it makes sense — that is, only when it eliminates repetition in the code.

Also notice that by using a multi-line string, even though it screws up my indentation, it helps to highlight an otherwise-easily-missed bug in the program!

I am excited about the opportunity to join {company_name} (insert additional details here).

Did you notice the (insert additional details here) stuffed in the middle of a string literal full of backslashes? You wouldn't want to send the email out like that, would you? :)


Is this method thread safe? Are  these methods getNewId() & fetchIdsInReserve() thread safe ?

public final class IdManager {

    private static final int NO_OF_USERIDS_TO_KEEP_IN_RESERVE = 200;

    private static final AtomicInteger regstrdUserIdsCount_Cached = new AtomicInteger(100);
    private static int noOfUserIdsInReserveCurrently = 0;

    public static int getNewId(){
          synchronized(IdManager.class){
              if (noOfUserIdsInReserveCurrently <= 20)
                      fetchIdsInReserve();

              noOfUserIdsInReserveCurrently--;
          }
          return regstrdUserIdsCount_Cached.incrementAndGet();
    }

    private static synchronized void fetchIdsInReserve(){
        int reservedInDBTill = DBCountersReader.readCounterFromDB(....); // read column from DB

        if (noOfUserIdsInReserveCurrently + regstrdUserIdsCount_Cached.get() != reservedInDBTill) throw new Exception("Unreserved ids alloted by app before reserving from DB");

        if (DBUpdater.incrementCounter(....)) //if write back to DB is successful
              noOfUserIdsInReserveCurrently += NO_OF_USERIDS_TO_KEEP_IN_RESERVE;
    }

}

Imagine this scenario: thread A arrives first and executes the entire synchronized(IdManager.class) block, but is interrupted before the row return regstrdUserIdsCount_Cached.incrementAndGet(); so that the counter is not incremented.

The arrives thread B. It enters the synchronized(IdManager.class) block and gets the same Id as thread A!!!

This could happen because you have put the regstrdUserIdsCount_Cached.incrementAndGet(); outside the syncronized block. If you put it inside, your code should be thread safe.

UPDATE:

The scenario described above is not possibile, because incrementAndGet is an atomic operation. Indeed, the code is thread-safe. My apologies.


Deep reference and object passing I'm curious if this is the right way of going about the issue:

function sortWordsByIncorrectAnswers(array) {
    array = array.slice(0)
    var sortedArray = array.sort(function (a, b) {
        a = a.fields.false;
        b = b.fields.false;
        return a < b ? -1 : a > b ? 1 : 0;
    });
    return sortedArray
}

I want to return a new version of the array, and not change 'array' passed into the method (deep copy).

This works as I want it to, but is this the correct way of doing this? It seems off.

.slice() does not actually do a deep copy, the easiest way to do a deep copy with simple objects is

array = JSON.parse( JSON.stringify( array ) )

Also, you can sort on a boolean in a simpler way:

function sortWordsByIncorrectAnswers(array)
{
  array =  JSON.parse( JSON.stringify( array || [] ) );
  return array.sort( function(a, b)
  {
    return a.fields.false - b.fields.false;
  });
}


Retrieve annotation from a tree annotated with a Monoid Working on this homework from 2013 for self-learning.

For the following Algebraic Data Type:

data JoinList m a = Empty
                  | Single m a
                  | Append m (JoinList m a) (JoinList m a)
    deriving (Eq, Show)

I implemented tag:

tag :: Monoid m => JoinList m a -> m
tag Empty                 = mempty
tag (Single x _)          = x
tag (Append x left right) = x

Example:

*JoinList Data.Monoid> tag (Append (Product 100) (Single (Product 10) "foo") (Single (Product 10) "bar"))
Product {getProduct = 100}

*JoinList Data.Monoid> tag (Empty :: JoinList (Product Integer) String)
Product {getProduct = 1}

*JoinList Data.Monoid> tag (Single (Product 5) "foo")
Product {getProduct = 5}

Originally, I had implemented the Append ... case as:

tag (Append x left right) = x  `mappend` (tag left) `mappend` (tag right)

But, I'm not sure if I can assume that the Monoid m in JustList m x is already populated.

Please critique my implementation.

Since this is a relatively simple question, 4 days is enough waiting for a more knowledgeable answer.

Your implementation of tag is correct and there is neither much room for improvement, nor there really is other ways to do it. You can replace Append x left right with Append x _ _ like you did for Single, and that's about the change you can make.

  Originally, I had implemented the Append ... case as:

That implementation was just wrong, consider what the two implementations return for your first example: 100 != 100 * 10 * 10.

  I'm not sure if I can assume that the [a] in [B] is
  already populated.

You can assume it is populated, you cannot have instances of algebraic data types with missing elements.

What you were trying to do probably is ensuring that an invariant, namely tag (Append x left right) == tag left <> tag right, is satisfied. This is the duty of a smart constructor. In this case the other function (+++ operator) you were asked to implement in the homework.


Python Hash Table implemented using lists I implemented a Python Hash Table using only lists.
Here is my implementation:

class HashTable:
    # Initialize the table with a fixed size of 54 slots
    def __init__(self):
        self.Table = [None] * 54

    def _get_value(self, key): #This is my hash function.
        # It finds the hash of every string. total % 54
        total = hash(key)
        return total % 54

    def insert(self, key):
        val = self._get_value(key)
        col = False #Collision bool
        index = 0

        if self.Table[val] == None: #Empty slot - turn into list of keys to avoid extra cases
            self.Table[val] = [key]

        else: #Collision - append
            self.Table[val].append(key)
            col = True
            index = len(self.Table[val]) - 1

        return val, col, index

    def delete(self, key):
        val = self._get_value(key)

        if self.Table[val] == None: #Deleting an unexisting element
            return -1, 0

        elif key in self.Table[val]: #This is the O(n) part of the hashtable
            index = self.Table[val].index(key)
            self.Table[val].remove(key)
            return val, index

        else: # No match was found in list, element does not exist
            return -1, 0

    def lookup(self, key):
        val = self._get_value(key)

        if self.Table[val] == None:
            return -1, 0
        else:
            if key in self.Table[val]:
                index = self.Table[val].index(key)
                return val, index

            # No match was found in list, element does not exist
            return -1, 0

    def clear(self):
        self.__init__()

I am using and returning some variables to keep track of collision, values, and indexes to print to the user.

I want to understand how I could improve this implementation - still using only lists (no sets for checking).
From a time complexity perspective or even code design, how can I make this more efficient? What design choices were poorly made in my code? What can I improve in this to make it better?

Restructuring and micro-optimization

Hard-coded table size 54 is better defined as class constant:

T_SIZE = 54

Encapsulate the crucial list/table at least as protected property self._Table.

Calling self._get_value(key) and the condition if self._Table[val] == None are repeated in most crucial methods. To reduce that noisy repetition an additional method can be defined which will return a tuple of calculated value val and is_empty ("empty slot" flag):

def _check_value(self, key):
    val = self._get_value(key)
    is_empty = self._Table[val] is None
    return val, is_empty

It doesn't make sense to construct if ... else conditional if the 1st if branch return 's immediately.

Both delete and lookup methods, on existing item, will perform 2 access/lookup operations on self._Table[val]:

key in self._Table[val]
self._Table[val].index(key)

To reduce access operations there we can apply try/except trick and returning the needed result in each separate block. See the final implementation below:

class HashTable:
    T_SIZE = 54

    # Initialize the table with a fixed size
    def __init__(self):
        self._Table = [None] * HashTable.T_SIZE

    def _get_value(self, key):
        total = hash(key)
        return total % HashTable.T_SIZE

    def _check_value(self, key):
        val = self._get_value(key)
        is_empty = self._Table[val] is None
        return val, is_empty

    def insert(self, key):
        val, is_empty = self._check_value(key)
        col = False  # Collision flag
        index = 0

        if is_empty:  # Empty slot - turn into list of keys to avoid extra cases
            self._Table[val] = [key]
        else:
            self._Table[val].append(key)  # Collision - append
            col = True
            index = len(self._Table[val]) - 1

        return val, col, index

    def delete(self, key):
        val, is_empty = self._check_value(key)
        if is_empty:  # Deleting unexisting element
            return -1, 0

        try:
            index = self._Table[val].index(key)
            self._Table[val].remove(key)
            return val, index
        except ValueError:  # No match was found in list, element does not exist
            return -1, 0

    def lookup(self, key):
        val, is_empty = self._check_value(key)
        if is_empty:
            return -1, 0
        try:
            index = self._Table[val].index(key)
            return val, index
        except ValueError:  # No match was found in list, element does not exist
            return -1, 0

    def clear(self):
        self.__init__()

In case if _get_value method won't be used in standalone context - you may easily inline it into _check_value method.

Sample usage:

h = HashTable()
h.insert('abc')
h.insert('abc')
print(h.lookup('abc'))
print(h.lookup('1abc'))
print(h.delete('abc'))
print(h.delete('abc'))
print(h.delete('abc'))

The output (consecutively):

(8, 0)
(-1, 0)
(8, 0)
(8, 0)
(-1, 0)


Converting from Roman numerals to Arabic numbers I got an idea to make a Roman/Arabic number converter, and I would like to hear your thoughts, where I could improve it, if there are Python specific things that I missed, so on and so forth.

# Decodes roman numerals
def get_arabic_numbers(t):
    # Hold the numbers in the list
    l = list()

    # The format
    format = 0

    # Convert from roman numeral to arabic number
    for i in range(0, len(t)):
        if t[i] == 'I':
            l.append(int(1))
        elif t[i] == 'V':
            l.append(int(5))
        elif t[i] == 'X':
            l.append(int(10))
        elif t[i] == 'L':
            l.append(int(50))
        elif t[i] == 'C':
            l.append(int(100))
        elif t[i] == 'D':
            l.append(int(500))
        elif t[i] == 'M':
            l.append(int(1000))

    # Calculate the format as follows:
    i = 0
    while i < len(l) - 1:
        # If the next numeral is greater than the current one, add the next one minus the current one = format = format + next_numeral - current_numeral
        if l[i] < l[i+1]:
            format = format + l[i+1] - l[i];
            i = i + 2
        # Else add it normally
        else:
            format = format + l[i]
            i = i + 1

    # Fix in case the last two numerals are equal
    if i == len(l):
        if l[i-2] == l[i-1]:
            format = format + l[i-1]
    else:
        format = format + l[i]

    # Return the format
    return format

# Encode an arabic digit to a roman numeral
def encode(l, c, one, five, nine):
    # The format to be returned
    format = ""

    # Special case 5.1: 5 has its own numeral 'V'
    # Special case 5.2: 50 has its own numeral 'A'
    # Special case 5.3: 500 has its own numeral 'D'
    if l[c] == 5:
        format = format + five
    elif l[c] < 5:
        for i in range(0, l[c]):
            format = format + one
    elif l[c] > 5:
    # Special case 9.1: 9 depends on 10, so it's 'IX'
    # Special case 9.2: 90 depends on 1000, so it's 'XC'
    # Special case 9.3: 900 depends on 1000, so it's 'CM'
        if l[c] == 9:
            format = format + nine
        else:
            format = format + five
            for i in range(6, l[c] + 1):
                format = format + one

    return format

# Encodes arabic numbers
def get_roman_numerals(t):
    # The arabic number to be converted
    t = int(t)
    # The list contains the number's digits
    l = list()

    # int is not iterable, so I wrote this to get the digits
    while t >= 1:
        l.append(int(t % 10))
        t = t / 10

    # They are in the wrong order, so the list has to be reversed
    l.reverse()

    # The format
    format = ""

    # Current position in the number
    c = 0

    # If the number is at least 1000
    if len(l) >= 4:
        num = 0

    # All the digits after 1000
    i = int(len(l) - 1)
    while i > 3:
        num = num + 1
        i = i - 1

    # A list containing these digits
    li = list()

    for i in range(0, num + 1):
        li.append(l[i])

    # Converts the list to an int
    n = map(str, li)
    n = ''.join(n)
    n = int(n)

    # Add an 'M' for every 1000
    for i in range(0, n):
        format = format + 'M'

    c = num + 1

    # If the number is also least 100
    if len(l) >= 3:
        format = format + encode(l, c, 'C', 'D', "CM")
        c = c + 1

    # If the number is at least 10
    if len(l) >= 3:
        format = format + encode(l, c, 'X', 'L', "XC")
        c = c + 1

    # If the number is at least 100 but smaller than 1000
    if len(l) > 2:
        format = format + encode(l, c, 'I', 'V', "IX")

    # If the number is at least 10 but smaller than 100
    if len(l) == 2:
        format = format + encode(l, c, 'X', 'L', "XC")
        c = c + 1
        format = format + encode(l, c, 'I', 'V', "IX")

    # If the number is at least 1 but smaller than 10
    if len(l) == 1:
        format = format + encode(l, c, 'I', 'V', "IX")

    return format

Impressions

That's a very long solution.  The comments were generally helpful, but the code itself could have been easier to understand if it were shorter and more expressive.

The variable names were generally not helpful: l, i, t, c, li were all cryptic.

get_arabic_numbers(t) is not really named appropriately:

"get" implies retrieving something that already exists.  This is more of a calculation.
Why is "numbers" plural?
Why is the parameter named t — what does it stand for?
Strictly speaking, the result is an int.  It's not Arabic, nor is it even in base ten.  It's just an abstract integer, which is conventionally rendered for you as base-ten Arabic digits when str() is called on it (whether explicitly or implicitly).  (And by "Arabic", we mean 0123456789, not ٠١٢٣٤٥٦٧٨٩.)

Based on these concerns, I'd call it decode_roman_numeral(roman).  It happens to be what you wrote as the comment!  Similarly, I'd call the inverse function encode_roman_numeral(num).

Arabic → Roman

# The format
format = 0

That's not what I would call a "format" — I would call it a "result".

# Hold the numbers in the list
l = list()
…

# Convert from roman numeral to arabic number
for i in range(0, len(t)):
    if t[i] == 'I':
        l.append(int(1))
    elif t[i] == 'V':
        l.append(int(5))
    elif t[i] == 'X':
        l.append(int(10))
    elif t[i] == 'L':
        l.append(int(50))
    elif t[i] == 'C':
        l.append(int(100))
    elif t[i] == 'D':
        l.append(int(500))
    elif t[i] == 'M':
        l.append(int(1000))

As I mentioned before, l is a cryptic name.  There is no need to convert 1 to int(1).  Lookups are typically done in Python using a dictionary.  Also, whenever you populate a list by creating an empty list and appending to it repeatedly, that is a good candidate for a list comprehension:

trans = {'I': 1, 'V': 5, 'X': 10, 'L': 50, 'C': 100, 'D': 500, 'M': 1000}
values = [trans[r] for r in roman]

# Calculate the format as follows:
i = 0
while i < len(l) - 1:
    # If the next numeral is greater than the current one, add the next one minus the current one = format = format + next_numeral - current_numeral
    if l[i] < l[i+1]:
        format = format + l[i+1] - l[i];
        i = i + 2
    # Else add it normally
    else:
        format = format + l[i]
        i = i + 1

Do you really need a case that advances i by two?  How about this instead:

result = 0
for i in range(0, len(l) - 1):
    if l[i] < l[i+1]:
        result -= l[i]
    else:
        result += l[i]

As I mentioned before, the code could be more expressive.  This loop is summing values in a list, so we should use the sum() built-in function to say what we mean.

result = sum(
    val if val >= next_val else -val
    for val, next_val in zip(values[:-1], values[1:])
)

All together, then, I'd condense your function down to this:

def decode_roman_numeral(roman):
    """Calculate the numeric value of a Roman numeral (in capital letters)"""
    trans = {'I': 1, 'V': 5, 'X': 10, 'L': 50, 'C': 100, 'D': 500, 'M': 1000}
    values = [trans[r] for r in roman]
    return sum(
        val if val >= next_val else -val
        for val, next_val in zip(values[:-1], values[1:])
    ) + values[-1]

Arabic → Roman

The encode() function encodes a single Arabic digit, so encode_digit() would be a better name.  There is no point in passing l and c separately, since you only ever care about l[c].  Instead of looping to append characters to format, use the * operator to construct a repeated string.

I would write this helper function using a single expression with four cases:

def encode_digit(digit, one, five, nine):
    return (
        nine                     if digit == 9 else
        five + one * (digit - 5) if digit >= 5 else
        one + five               if digit == 4 else
        one * digit
    )

The get_roman_numerals() function is rather convoluted.  I had a hard time following the code, so I went by your comments instead.  In the end, I figured out that it was essentially doing this:

def encode_roman_numeral(num):
    num = int(num)
    return (
        'M' * (num // 1000) +
        encode_digit((num // 100) % 10, 'C', 'D', 'CM') +
        encode_digit((num //  10) % 10, 'X', 'L', 'XC') +
        encode_digit( num         % 10, 'I', 'V', 'IX')
    )


Integer to binary using an old-school way I know I could achieve this using System.out.println(Integer.toBinaryString(num)); in Java. However, I wanted to do it this way.

private static void toBinary(int x) {

    Scanner in = new Scanner(System.in);
    int num = in.nextInt();
    String s = "";

    while (num >= 2) {

      s = s + valueOf(num % 2);

      num = (int) num / 2;
    }

    String newS = new StringBuilder(s).reverse().toString();

    System.out.print(num + newS);
  }

Functional extraction

Your method does not do toBinary, it does:

read input from user
convert to binary
print back for user

When you have methods that do multiple things it becomes hard to isolate and reuse the logic. Your code should be split in to parts, and the toBinary method should take the int input and return a String

Complicated end-of-loop handling

The while condition while (num >= 2) { is a complicated construct, that needs a comment. That logic is key to the process, and guarantees that you are left with a "dangling" 1 for the high-bit, and also handles the 0 input case. It took me a number of minutes to figure out how that code works, and that's not useful for your readers... (or yourself, a year in the future).

As it happens, a do-while loop is the structure you want, and it simplifies the logic a whole bunch....

valueOf(...) mystery

I assume you have done a static import of the String class? Otherwise the valueOf method is not defined. Doesn't this defeat some of your "old-school" desire? Also, why not just use a simple 2-value lookup table. The digits are either 0 or 1, so it's simply:

char[] digits = new char[]{'0', '1'};

and then your valueOf can be replaced with:

digits[num % 2]

implicit type conversion

You do an implicit type conversion in the output num + newS which is not "old-school" either. Adding a number to a string is... complicated.

Unnecessary reversals

Why do the reverse mechanism? Why not just build the digit string up backwards to start with? Instead of:

s = s + valueOf(num % 2);

have:

s = valueOf(num % 2) + s;

String concatenation

String concatenation with the + operator is often frowned on... in this instance, it makes the code cleaner, and I am leaving it in. The alternative is to use a StringBuilder, but that has other issues in this use-case. Like you, I would use string concatenation...

Conclusion

All told, I would recommend something like:

private static final char[] digits = "01".toCharArray();

private static String toBinaryRL(int num) {
    if (num < 0) {
        throw new IllegalArgumentException("toBinary can only handle positive integers, not " + num);
    }
    String binary = "";
    do {
        binary = digits[num % 2] + binary;
        num /= 2;
    } while (num > 0);
    return binary;
}

Further, this process can be easily extended in to any "base", by extending the digits, and adding a parameter to the function:

private static final char[] digits = "0123456789abcdefghijklmnopqrstuvwxyz".toCharArray();

private static String toBase(int num, int base) {
    if (num < 0) {
        throw new IllegalArgumentException("toBase can only handle positive integers, not " + num);
    }
    if (base <= 1 || base >= digits.length) {
        throw new IllegalArgumentException("Cannot convert to base " + base);
    }
    String result = "";
    do {
        result = digits[num % base] + result;
        num /= base;
    } while (num > 0);
    return result;
}

private static String toBinaryB(int num) {
    return toBase(num, 2);
}


Creating a Python namedtuple class that forces some of its fields to be floats I've got a use case for namedtuple that requires some of the fields to be floats. I could require the code that's constructing the namedtuple to supply floats, but I'd like for the namedtuple class to be smart enough to convert strings it gets into floats for me (but only for a specified subset of fields). I have code to do this, but it's ugly:

from collections import namedtuple, OrderedDict

# This list is built at runtime in my application
_FIELDS_AND_TYPES = [("num1", True), ("num2", True), ("label", False)]
FIELDS_FORCE_FLOAT = OrderedDict(_FIELDS_AND_TYPES)

_MyNamedTuple = namedtuple("_MyNamedTuple", FIELDS_FORCE_FLOAT.keys())
class TypedNamedTuple(_MyNamedTuple):
    fields_force_float = FIELDS_FORCE_FLOAT
    def __new__(cls, *values_in):
        super_obj = super(_MyNamedTuple, cls)
        superclass = super_obj.__thisclass__
        values = [float(value) if cls.fields_force_float[fieldname] else value
                    for fieldname, value in zip(superclass._fields, values_in)]
        self = super_obj.__new__(cls, values)
        return self

print TypedNamedTuple("1.0", "2.0", "3.0")
# _MyNamedTuple(num1=1.0, num2=2.0, label='3.0')

Things I don't like about this code:

The output of print TypedNamedTuple("1.0", "2.0", "3.0") starts with _MyNamedTuple rather than TypedNamedTuple. I think this is a general problem with subclassing namedtuples, so fair enough. I could give both classes the same name to solve this.
My code has to pull in FIELDS_FORCE_FLOAT from a global variable.
TypedNamedTuple is inefficient (it runs zip and does a bunch of dictionary lookups). This is not so bad in my context but I'd like to know how to handle this "right."
If I wanted to create another namedtuple subclass that forces some of its args to be float, I'd basically be starting from scratch. My TypedNamedTuple is not reusable.

Is there a cleaner way to do this?

As for why I'm attempting to use namedtuples here at all (H/T @jonrsharpe), it's not set in stone but here are some things that pushed me in this direction:

The list of fields I'm interested in isn't known until runtime.
Immutability: I do not, in fact, have a reason to change any of these objects once they're constructed.

In order to reuse code (that is, namedtuple) it should be okay to use
this approach.  It might also make things harder in the long run, so it
could be easier to use a regular meta-/class or decorator to enforce this kind of
logic.  I also see a problem with automatic conversion of values, as
that needs to be communicated/debugged later on, but if it fits the use
case, why not.  Since the tuple is immutable there's no normal way to
circumvent this, so I guess it is less of an issue than with a mutable
class.

Also, have you looked at the collections.py source?  I mean it's good
that it works, but honestly that code generation is a bit scary.  It is, however, a way to generate all this stuff at runtime.  If you don't have to have separate Python classes you could also just use a single class and pass in the valid fields as a separate parameter, then use functions to create the separate tuple "types".

For the presented code / your questions I have some other suggestions:

You can override __repr__ to fix the "wrong" printed
representation.  I'm not quite sure exactly why it is kind of
hardcoded in collections.py, but generally I'd prefer things to
print their own class name instead, as written below.  The
formatting is copied from the standard namedtuple.
If it's happening once, that is not nice, but certainly not the
biggest issue.  Again, you could follow the code generation model
and pass it in as a parameter,
e.g. typednamedtuple('TypedNamedTuple', FIELDS_AND_TYPES).
Use izip to prevent allocating a new list.  Otherwise looks good?
I can't see how you would otherwise do the conversion.
Subclass and override the field?  Note the AnotherTypedNamedTuple
below.  Otherwise see above.

Also:

Don't assign to self.  That looks super wrong.

from collections import namedtuple, OrderedDict
from itertools import izip

# This list is built at runtime in my application
_FIELDS_AND_TYPES = [("num1", True), ("num2", True), ("label", False)]
FIELDS_FORCE_FLOAT = OrderedDict(_FIELDS_AND_TYPES)

_MyNamedTuple = namedtuple("_MyNamedTuple", FIELDS_FORCE_FLOAT.keys())
class TypedNamedTuple(_MyNamedTuple):
    fields_force_float = FIELDS_FORCE_FLOAT

    def __new__(cls, *values_in):
        super_obj = super(_MyNamedTuple, cls)
        superclass = super_obj.__thisclass__
        values = [float(value) if cls.fields_force_float[fieldname] else value
                  for fieldname, value in izip(superclass._fields, values_in)]
        return super_obj.__new__(cls, values)

    def __repr__(self):
        return "{}({})".format(
            self.__class__.__name__,
            ', '.join("{}=%r".format(name) for name in self._fields) % self)

print TypedNamedTuple("1.0", "2.0", "3.0")
# TypedNamedTuple(num1=1.0, num2=2.0, label='3.0')

class AnotherTypedNamedTuple(TypedNamedTuple):
    fields_force_float = OrderedDict([("num1", False), ("num2", False), ("label", False)])

print AnotherTypedNamedTuple("1.0", "2.0", "3.0")
# AnotherTypedNamedTuple(num1='1.0', num2='2.0', label='3.0')


More elegant filter script in R I'm very new both to programming and to R so please bear with me :-)
I've written the following bit of code, which works perfectly well and runs through a data file with 17446 rows in about 35 seconds. I don't really have a problem with this but am sure that this could be a lot more elegant with probably the use of the tapply function. I would love to see how you experts would rewrite this more efficiently and am sure the rewrite would teach me a lot. I've included the first few rows of the output file and hopefully the code should be fairly obvious (it must be if I managed to write it!!); it's a simple filter based on the standard deviation of a subset in one column dictating the output to another column which is set up at the start of the code. If a value in the NEE column is more than 2 stdevs of the preceeding three values then the value is taken from the mean of a subset in the NEE2 column else the value is copied from the first (if that makes sense). Please also note the "count" variable as I need to retrieve the amount of values replaced. Hope this piques someone's interest and thanks in advance for all of your time.
JonP

Dspke <- read.csv(file.choose())
nr = nrow(Dspke)
count = 0

Dspke$NEE2 <- (1:nr) #creates a new column ready for input of values

for (i in 4:nr) {

#standard deviation of the previous three values in NEE
stdev <- sd(Dspke$NEE[(i-2):i])

#if stdev>2 then NEE2 value is mean of the previous 3 values in NEE2 else copy value from NEE
if(stdev>=2) {
  Dspke$NEE2[i] <- mean(Dspke$NEE2[(i-2):i-1])
    count = count+1

    }else { Dspke$NEE2[i] <- Dspke$NEE[i]
  }

 }
  write.csv(Dspke,"Dspke.csv")

 Date_Time       NEE     NEE2
1 03/01/2012 13:00 -2.300000  1.00000
2 03/01/2012 13:30 -2.385610  2.00000
3 03/01/2012 14:00 -2.081935  3.00000
4 03/01/2012 14:30 -1.778260 -1.77826
5 03/01/2012 15:00 -2.409490 -2.40949
6 03/01/2012 15:30 -0.741030 -0.74103

The embed function is well-suited for constructing trailing vectors by row:

 Dspke[ , paste0("E3", 1:3)] <- NA     # create columns to hold the trailing values
 Dspke[4:nrow(Dspke), 5:7] <- embed(Dspke$NEE, 3)
 Dspke$NEE3 <- ifelse( apply(Dspke[,5:7], 1, sd) >2,   #test
                             rowMeans(Dspke[,5:7]),   # result if TRUE
                             Dspke[,"NEE"])           # result if FALSE

I see the count vector being created but cannot tell from your description what it is supposed to do. Perhaps you should store the value of:

> apply(Dspke[,5:7], 1, sd) > 2)
    1     2     3     4     5     6
   NA    NA    NA FALSE FALSE FALSE

(Not a very good test of the code presented by the data since all of the tests are FALSE.)


Chatroom log in with no 3rd party libraries So, I'm creating a chatroom with no 3rd party libraries and currently I'm logging people in like this:

public void run() {
    try {
        this.running = true;
        ServerSocket ss = new ServerSocket(1000);
        while (this.running) {
            this.threadPool.execute(new InfoGrabber(ss.accept()));
        }
    } catch (IOException ex) {
        Logger.getLogger(Listener.class.getName()).log(Level.SEVERE, null, ex);
    }
}

private class InfoGrabber implements Runnable {

    private final Socket socket;

    public InfoGrabber(Socket socket) {
        this.socket = socket;
    }

    @Override
    public void run() {
        try {
            ObjectInputStream ois = new ObjectInputStream(socket.getInputStream());
            Object obj = ois.readObject();
            if (obj instanceof ClientInformation) {
                clientManager.addClient(new ClientClass((ClientInformation) obj, new ClientConnection(socket, ois, ((ClientInformation)obj).getUUID())));
                System.out.println("Client connected");
            }
        } catch (IOException | ClassNotFoundException ex) {
            Logger.getLogger(Listener.class.getName()).log(Level.SEVERE, null, ex);
        }
    }
}

Is there a better way to do this?
Git for more context https://github.com/CjStaal/ChatRoom
I have a systemQueue set up which takes SystemMessage which is a string, and an object. I want to be able to make this better, and have it more encapsulated and modular to allow easier maintainability and expandability. In the long run, I want it to be able to authenticate against a SQL database.

You have a number of missing links in your code, which makes a review hard to do.

Your ServerSocket code at first glance looks OK, but it is actually broken.

    this.running = true;
    ServerSocket ss = new ServerSocket(1000);
    while (this.running) {
        this.threadPool.execute(new InfoGrabber(ss.accept()));
    }

You don't have a declaration for running. If it is a volatile boolean, then it will probably work. If it is not volatile, it'll possibly never work, depending on the setup. I would never recommend the use of volatile, though, so I would recommend an AtomicBoolean. Still, even with an atomic boolean, you may never exit your server loop because it only checks to see if it should stop after it has accepted a connection. The code will sit waiting on the accept() until something happens, even if the running is false. You need a different mechanism, or timeouts, on your code.

The actual socket you get is also badly handled. You create a thread to run the socket on, and you create an ObjectInputStream on the socket. This is OK, but you then schedule that on a thread which exits quickly... the ClientClass instance is registered on to the clientManager, but then no thread appears to be listening for more input from the client? Additionally, nothing has told the client that it is connected.

This code appears to be pretty new, and incomplete.


Ruby singleton for logging I currently have a logging setup that looks like this:

require 'active_support'
require 'singleton'
class Worker
  def initialize(logger)
    @logger = logger
  end

  def perform
    @logger.with_context(["tag1"], "Started work")
    Provider.say_hello
  end
end

class Vlogger
  include Singleton
  attr_accessor :logger

  def logger
    @logger ||= ActiveSupport::TaggedLogging.new(Logger.new(STDOUT))
  end

  def with_context(tags, msg)
    @logger.tagged(*tags) { @logger.info msg }
  end
end

class Provider
  def self.logger
    Vlogger.instance
  end

  def self.say_hello
    logger.with_context(["tag2"], "hello")
  end
end

v = Vlogger.instance
v.logger = ActiveSupport::TaggedLogging.new(Logger.new(STDOUT))
Worker.new(v).perform

My objective with this is to avoid having to pass around the @logger instance variable found in my Worker class.  Ideally, I want the Vlogger class to be available to all classes.  I'm finding that I'm having to use this pattern quite a bit in my code:

  def self.logger
    Vlogger.instance
  end

Is there a better way to do this?  Is my OO design pattern here inherently flawed? Ideally I'm trying to avoid passing around objects when really the object has no state and it should be available to various classes without introducing class or instance methods.  Thoughts?

You might be better off just using a module with some static methods, e.g.:

module Vlogger
  class << self
    def logger
      @logger ||= ActiveSupport::TaggedLogging.new(Logger.new(STDOUT))
    end

    def with_context(tags, message)
      logger.tagged(*tags) { logger.info message }
    end
  end
end

Add attribute access as needed.

Now you can simply call Vlogger.with_context directly when you need it without having to use instance or otherwise keeping a reference to things elsewhere.

class Worker
  def perform
    Vlogger.with_context(["tag1"], "Started work")
    Provider.say_hello
  end
end

class Provider
  def self.say_hello
    Vlogger.with_context(["tag2"], "hello")
  end
end

It's an option, at least.


Finding duplicate numbers I have written this code which finds duplicate numbers and the occurrence of a particular number in an array. I have used a HashMap, but I want to know if there is a more efficient way to do the same, or whether I should be using another method.

import java.util.*;
class test7
{
    public static void main(String ...a)
    {
        int []arr={10,20,10,2,11,10,32,15,15,10,10};
        HashMap<Integer,Integer> num=new HashMap<Integer,Integer>();
        for(int t: arr)
        {
            Integer tmp_int=new Integer(t);
            if(num.containsKey(tmp_int))
            {
                Integer i_ob=num.get(tmp_int);
                num.put(tmp_int,new Integer(i_ob.intValue()+1));
            }
            else
            {
                num.put(tmp_int,new Integer(1));
            }
        }
        System.out.println(num);
    }
}

Output:

{32=1, 2=1, 20=1, 10=5, 11=1, 15=2}

I have modified your code. The logic is same as you have done. You do not need to create those Integer. Read about Autoboxing. If the input array is large, you will see performance hit for creating Integer. Note that Integer is a class, and not a primitive type in java.

public static void main(String[] args) {
    int[] arr={10,20,10,2,11,10,32,15,15,10,10};
    HashMap<Integer,Integer> result = new HashMap<Integer,Integer>();

    for(int element : arr)
      {
        if(result.containsKey(element))
          {
            result.put(element, result.get(element) + 1);
          }
        else
          {
            result.put(element, 1);
          }
      }
    System.out.println(result);
  }