Simple Scrabble Game Scorer I am making a program that scores a scrabble game and this is the basic version of it.  Eventually I am going to add logic for double and triple words and letters but before I get too ahead of myself I would like to improve the basic code here.

All suggestions are welcome.

def main():
    letter_val = {" ": 0, "a": 1, "b": 3, "c": 3, "d": 2, "e": 1, "f": 4, "g": 2, "h": 4, "i": 1, "j": 8, "k": 5, "l": 1, "m": 3, "n": 1, "o": 1, "p": 3, "q": 10, "r": 1, "s": 1, "t": 1, "u": 1, "v": 4, "w": 4, "x": 8, "y": 4, "z": 10}
    players = []
    """ Function to add as many players as there are in the game (uses scrabble rules for limit) """
    def add_players():
        while True:
            while len(players) < 4:
                pla = input("Enter Player Names (can have up to 4) >>> ")
                if pla:
                    if pla not in (a for i in players for a in i):
                        if len(players) < 4:
                            players.append([pla, 0])
                            print("Player {} added".format(pla))
                        else:
                            return
                    else:
                        print("Name already in players' list")
                else:
                    return
            else:
                return

    def home(undo_ind=False):
        option = input('Would you like to [A]dd a score, [V]iew scores, [U]ndo the last change, or [End] the game? > ')

        class Score:
            def __init__(self):
                global temp_v, temp_p
                player = temp_p = input("Enter player to score>>> ")
                if player in (a for i in players for a in i):
                    try:
                        word = input("Enter word to score>>> ")
                        value = temp_v = sum(letter_val[i.lower()] for i in word)
                    except KeyError:
                        print("Word must consist of letters only.")
                        Score()
                    for i in players:
                        if i[0] == player:
                            i[1] += value
                else:
                    print("Player entered is not in player list.")
                home()

            @staticmethod
            def undo():
                try:
                    for i in players:
                        if i[0] == temp_p:
                            i[1] -= temp_v
                            home(True)
                except NameError:
                    print("No changes have been made.")
                    home()

            @staticmethod
            def view_scores():
                for i in players:
                    print("Player %s has a score of %d" % (i[0], i[1]))
                home(undo_ind)
        if option.lower() == "a":
            Score()
        elif option.lower() == "v":
            Score.view_scores()
        elif option.lower() == "u" and undo_ind is False:
            Score.undo()
        elif option.lower() == "u" and undo_ind is True:
            print("No changes have been made.")
            home(True)
        elif option.lower() == "end":
            print("Final scores are:")
            for i in players:
                print("Player {} has a final score of {}.".format(i[0], i[1]))
        else:
            print("That is not a valid option.")
            home(undo_ind)
    add_players()
    home()

if __name__ == "__main__":
    main()

I apologize that this answer became a "stream of consciousness"-type answer. I found it difficult to properly categorize what I wanted to say.

There's a fair amount to be said in add_players. The way you have things set up is quite confusing. You have fairly deep nesting, and multiple returns at multiple different levels of nesting.

if pla not in (a for i in players for a in i) is also confusing. Think about for i in players. You're referring to each player as i? Also, your intent with that generator expression seems to be to flatten players so you can check if a name already exists. Flattening doesn't seem necessary though since each "player" is a name and score. Why check if a name is equal to a score?

You also have a while len(players) < 4 inside of a while True. I can't really see the point of the while True though.

I would also use much more descriptive names. Once you reduce the nesting, you'll have a lot more room to work with.

Finally, add_players really shouldn't be adding to a global players. players should be returned from add_players and assigned at the call site.

Altogether, I'd write this closer too:

def add_players():
    players = []

    while len(players) < 4:
        new_name = input("Enter Player Names (can have up to 4) >>> ")

        if new_name:
            if new_name not in (name for name, _ in players):
                players.append([new_name, 0])
                print("Player {} added".format(new_name))

            else:
                print("Name already in players' list")

        else:
            break

    return players

Notice now there's a single return, and it's returning the players for the caller to use.

Please don't take this the wrong way, but the design of the rest of the program doesn't make much sense:

Why is everything inside of main? Ideally, the main function should be a small function at the end of your program that just calls a few other functions.
Why is a Score class inside of the home function?
Score also doesn't seem like it should even be a class. All of the methods of the class are static, and it seems like you're only using Score to score a word by calling the Score constructor. If you never use an instance of a class, it shouldn't be a class. Just make Score into a score_word function. I'd also pass a word into the function instead of asking inside of the function. Functions are harder to test when they produce their own data.
You're catching a NameError in undo. I'm guessing this is in case Score() hadn't been called yet? Don't do this. I can't think of a time when it's ever appropriate to catch a NameError. Any time a NameError happens, it means you have flawed logic in your program. You should fix the issue instead of putting an try/except band-aid over it. If the function requires temp_p and temp_v (which need better names), you need to ensure that that data is available. Either pass it in, or ensure that they're properly initialized ahead of time.

There's more that can be dug out here, but I need to start getting ready for work. I'll just make a few broad suggestions:

Please take much more care when writing code. A lot of this code seems like it was written quickly without much second-thought. I would focus on the intent of the code before it's written.

Ask yourself, "What exactly should this code do?", and "What are the appropriate tools to do what I'm trying to do?". Doing things like using classes as normal functions and writing code like (a for i in players for a in i) that technically works (but doesn't really do exactly what you want) makes your code hard to understand, and will make it difficult to add to later.
Don't nest everything. Really, all of these functions should be "top-level", outside of everything else. If you nest thing A inside of thing B, that suggests that A is deeply connected to B and wouldn't have a valid meaning outside of B. Really though, that's not the case here. Score would have the same meaning even if it were outside of home, and all the functions would have the same meaning even if they were outside of main.

Nesting conveys a certain association meaning, and if used improperly, gives the reader the wrong initial idea about how the code works. Nesting also forces you to use more indentation, which generally makes code harder to read.

With the above criticisms out of the way, I'll note what is good here:

You're following proper naming guidelines. You have functions starting with lowercase, classes starting with uppercase, and _ to separate "words" in names.
You're using idiomatic shortcuts like if pla: to check if a collection is empty or not.
You tried to have all the functionality start from a main instead of being loose in the script. This makes code easier to run and test.
letter_val is a good use of a dictionary.

I'll note though that it's a constant, so it should be in all uppercase.

Keep at it. There's a lot to be improved here, but there's promise too.


Ruby function to find numbers whose sum of squares of divisors is a perfect square I am doing a problem my buddy gave to me and I have my solution which works but needs to be optimised. I have tried to optimise it as much as I can with my knowledge but it seems there is still room for improvement. I am trying to improve my ruby. Here is the challenge:

Divisors of 42 are : 1, 2, 3, 6, 7, 14, 21, 42. These divisors squared are: 1, 4, 9, 36, 49, 196, 441, 1764. The sum of the squared divisors is 2500 which is 50 * 50, a square!

Given two integers m, n (1 <= m <= n) we want to find all integers between m and n whose sum of squared divisors is itself a square. 42 is such a number.

The result will be an array of arrays, each subarray having two elements, first the number whose squared divisors is a square and then the sum of the squared divisors.

 def list_squared(m, n)
   array = []
  (m..n).to_a.each do |i|
     q =  (1..i/2).to_a.push(i).map {|x| x**2 if i % x == 0 }.compact.reduce(:+)
     array << [i, q] if Math.sqrt(q) % 1 == 0
   end
 array
end

This works but apparently is inefficient. Could someone point me in the right direction. I am assuming its the "map" but I initially had:

def list_squared(m, n)
 array = []
 (m..n).to_a.each do |i|
   q =  (1..i/2).to_a.select {|x| i % x == 0 }.push(i).map { |x| x**2 }.reduce(:+)
    array << [i, q] if Math.sqrt(q) % 1 == 0
  end
 array
end

You don't need to call to_a.  Ranges are directly Enumerable.

Divisors always occur in pairs.  You don't need to test (1..i/2); you only need to go up to \$\sqrt{i}\$.

For fast performance, though, you would need a solution based on number theory rather than brute-force enumeration.


How can I find out why this Rust program to echo lines from stdin is slow? Given the following Rust program:

fn main() {
    let mut reader = io::stdin();
    for line in reader.lock().lines() {
        match line {
            Ok(l) => print!("{}", l),
            Err(_) => continue,
        }
    }
}

yes | program achieves 1.04MiB/s throughput, according to pv. The trivial C program below, which I fully realize does less, gives me a throughput of 141MiB/s.

int main() {
    int c;
    while((c=getchar()) != EOF)
        putchar(c);
}

How do I go about finding out what's keeping the Rust version from being faster? While I would appreciate if you just told me what to change to make it faster, I'm far more interested in how to find out what needs to be changed.

Edit: I tried changing lines() to chars() in the Rust version, to approximate the C version better, but it didn't seem to make any difference.

I'm going to treat this as a code review question where the goal is to figure out how to write high-performance I/O code in Rust.

There are two critical steps to speed up Rust I/O:

Make sure the optimizer is turned on. Rust loops have terrible performance in debug mode. Turning the optimizer on should probably get you near 25 MB/sec or so, at least in my experience.
Avoid line-based I/O, or anything else which uses String values. For maximum performance, we want to work with raw byte buffers and avoid ever letting Rust call malloc or free.

Here's some code which drills down to fill_buf, which is about as low as we can go before losing generality:

use std::io;
use std::io::{IoError, IoErrorKind};

fn main() {
    let mut reader = io::stdin();
    let mut buffer = reader.lock();
    let mut writer = io::stdout();
    loop {
        let consumed = match buffer.fill_buf() {
            Ok(bytes) => { writer.write(bytes).unwrap(); bytes.len() },
            Err(IoError{kind: IoErrorKind::EndOfFile, ..}) => break,
            Err(ref err) => panic!("Failed with: {}", err)
        };
        buffer.consume(consumed);
    }
}

Of course, this means that you need to find the line breaks yourself, and be prepared for lines to be split over multiple buffers. Also, notice how I pass bytes.len() outside of the match block before trying to call buffer.consume(). This is necessary to placate the lifetime checker.

Trying this with pv:

cat /dev/zero | target/release/throughput | pv > /dev/null

…gives us a throughput of 527 MB/s.


Complete missing data in timeseries pandas I collected data using a smartphone. The phone could run out of battery or the collection software could fail, leading to missing data.

For this example let's say I already compute the number of calls per day calls_per_day from the smartphone data. Besides the days where I actually call someone, I want to consider the days where I did not. To do this I have to consider the time where the phone was sensing and assign a zero to that day if there were no calls. In order to know if the phone was sensing, I have a reference sensor that records a tick every 25 seconds: sensed_time.

The code below does what I just described. First, it groups sensed_time by the minute, hour, and day, labelling a day as 'sensed' if it has collected at least 80% minutes of at least 80% of each of the 24 hours of a day. Then it just filters the days from calls_per_day that are labeled as sensed and replaces NaN values with 0.

import numpy as np
def count_per_minute(group):
    if group[group.columns[0]].count() == 0:
        return pd.Series({'count_per_minute':np.nan})
    else:
        return pd.Series({'count_per_minute':group[group.columns[0]].count()})

def label_sensed_days(group, hours_in_day, percentage_valid_hours, percentage_valid_minutes):
    sensed_hours_count = group['sensed_minutes'].loc[group['sensed_minutes'] > 60 * percentage_valid_minutes].count()

    if  sensed_hours_count > 24 * percentage_valid_hours:
        return pd.Series({'sensed_day':True, 'sensed_hours':sensed_hours_count})
    else:
        return pd.Series({'sensed_day':False, 'sensed_hours':sensed_hours_count})

# Create fake DF with the timestamps where the phone was sensing
index = pd.date_range("2018-01-01", "2018-01-03 23:59", freq='25S')
sensed_time = pd.DataFrame(index=index, columns=['was_sensed'])
sensed_time = sensed_time.fillna(1)
sensed_time = sensed_time.sample(frac=0.6)

# Count records sensed per minute, return nan if count == 0
sensed_minutes = sensed_time.groupby(pd.Grouper(freq='1Min')).apply(count_per_minute)

# Complete missing minutes
sensed_minutes = sensed_minutes.reindex(pd.date_range(sensed_time.index.min().date(), sensed_time.index.max().date() + pd.DateOffset(1), freq='1Min'))

# Group sensed minutes by hour
sensed_hours = sensed_minutes.groupby([pd.Grouper(freq='1H')]).count()
sensed_hours = sensed_hours.rename(columns={'count_per_minute':'sensed_minutes'})

# Group sensed hours per day but only consider a valid day the ones where at least 0.8 percent of 24 hours were sensed with at least 0.8 percent of minutes
sensed_days = sensed_hours.groupby([pd.Grouper(freq='1D')]).apply(label_sensed_days, hours_in_day=24, percentage_valid_hours= 0.8, percentage_valid_minutes=0.8)

# Create fake DF with the number of calls on every other day
index = pd.date_range("2018-01-01", "2018-01-05", freq='2D')
calls_per_day = pd.DataFrame([10,5,8], index=index, columns=['calls'])

# Only keep the days that we consider valid sensed days
calls_per_day = calls_per_day.reindex(sensed_days[sensed_days['sensed_day'] == True].index)

# All the NaN values mean that the phone was sensing but we didn't record a call, thus there were 0 calls.
calls_per_day = calls_per_day.fillna(0)

display(calls_per_day)

Don't forget you can chain operations (btw you missed the pandas import):

sensed_time = pd.DataFrame(index=index, columns=['was_sensed'])
sensed_time = sensed_time.fillna(1)
sensed_time = sensed_time.sample(frac=0.6)

x = pd.DataFrame(index=index, columns=['was_sensed']).fillna(1).sample(frac=0.6)

You can verify if this works by doing a sensed_time.shape == x.shape

Regarding the construction of your code, it seems like it would be a better idea to have the data object as a class, and get properties from it.

For instance, something along the lines of:

    class SmartPhoneData(object):
        def __init__(self, name=None):
            self.name = name or "Nokia6_32GB"
            self.data = pd.DataFrame(index=index, columns=['was_sensed']).fillna(1).sample(frac=0.6)

        def calls_per_day(self):
            print(f"SmartPhone Data: {self.data}")

        @property
        def minutes(self):
            return self.data.reindex(pd.date_range(sensed_time.index.min().date(), sensed_time.index.max().date() + pd.DateOffset(1), freq='1Min'))

        @property
        def hours(self):
            return self.data.groupby([pd.Grouper(freq='1H')]).count()

        @property
        def days(self):
            return self.data.groupby([pd.Grouper(freq='1D')]).apply(label_sensed_days, hours_in_day=24, percentage_valid_hours=0.8, percentage_valid_minutes=0.8)

    your_phone = SmartPhoneData()
    my_phone = SmartPhoneData("iPhone X")

    your_phone.calls_per_day()
    my_phone.calls_per_day()

Obviously I just took a single line from your code where it was similarly named (to match the function).
So if you do this - make sure you know how the data gets changed when you run each function. For instance, each of the @properties just does a return and doesn't modify the DataFrame, however some of your options might actually change the data contents (even if there is no assignment (e.g.: x = a + b doesn't change a or b, but a (not real code) self.data(column_names = ["one", "two"]) will change it). I hope that made sense?

Good Luck!


Trying to Store EditText Values in SharedPrefs using a Single addTextChangedListener The code  is completely functional but I'd rather not call two functions when I know it can be done more efficiently. My goal is to store EditText values in SharedPrefs using a single addTextChangedListener.  Any advice is much appreciated!

fun preserveText() {

    val sharedPref: SharedPreferences = PreferenceManager.getDefaultSharedPreferences(this)

    field_1.setText(sharedPref.getString("autoSave", ""))
    field_1.addTextChangedListener(object : TextWatcher {
        override fun beforeTextChanged(s: CharSequence, start: Int, count: Int, after: Int) {

        }

        override fun onTextChanged(s: CharSequence, start: Int, before: Int, count: Int) {

        }

        override fun afterTextChanged(s: Editable) {
            sharedPref.edit().putString("autoSave", s.toString()).apply()

        }
    })
}

fun preserveText2() {

    val sharedPref: SharedPreferences = PreferenceManager.getDefaultSharedPreferences(this)

    field_2.setText(sharedPref.getString("autoSave2", ""))
    field_2.addTextChangedListener(object : TextWatcher {
        override fun beforeTextChanged(s: CharSequence, start: Int, count: Int, after: Int) {

        }

        override fun onTextChanged(s: CharSequence, start: Int, before: Int, count: Int) {

        }

        override fun afterTextChanged(s: Editable) {
            sharedPref.edit().putString("autoSave2", s.toString()).apply()

        }
    })
}

I'm not sure if what I write below is what you are looking for.  Also I don't know much about Android, so maybe that would not work for some reason.

preserveText(field1, "autoSave")
preserveText(field2, "autoSave2")

val sharedPref: SharedPreferences = PreferenceManager.getDefaultSharedPreferences(this)

fun preserveText(field: Type????, itemName: String) {
    field.setText(sharedPref.getString(itemName, ""))
    field.addTextChangedListener(object : TextWatcher {
        override fun beforeTextChanged(s: CharSequence, start: Int, count: Int, after: Int) { }

        override fun onTextChanged(s: CharSequence, start: Int, before: Int, count: Int) { }

        override fun afterTextChanged(s: Editable) {
            sharedPref.edit().putString(itemName, s.toString()).apply()
        }
    })
}


Haskell/Yesod - Butchering use of MaybeT The following is some code that is used to provide the frontend with certain parameters when rendering a form. The parameters are conditional based around what data is being changed, getNewSaleFormOptions provides a way of quering these options using a GET request and url params (ie /sales/new?dealership=11&vehicle=2), and another getEditSaleFormOptions when returning to edit, but through the entity id, ie /sales/1 (this will pull necessary data, dealership, vehicle, from Sale 1).

I've tried to use MaybeT here to simplify a lot of the calls that return m (Maybe a), though its possible i'm not using it in the most efficient way. There are lots of MaybeT and runMaybeT calls (though this might be okay), and a few spots where there is duplicated code... I'm just not sure of the best way to eliminate them.

{-# LANGUAGE GADTs            #-}
{-# LANGUAGE FlexibleContexts #-}

module Forms.SaleForm
  ( SaleFormOptions(..)
  , getNewSaleFormOptions
  , getEditSaleFormOptions
  ) where

import           Control.Monad (join)
import           Control.Monad.Trans.Maybe (MaybeT(..), runMaybeT)
import qualified Data.Aeson as Aeson
import           Data.Aeson ((.=), ToJSON)
import qualified Data.Maybe as M
import           Data.Text (Text)
import           Database.Esqueleto (SqlPersistT, get)
import           Yesod.Core (MonadHandler(..), PathPiece(..))

data SaleFormOptions = SaleFormOptions
  { saleFormOptionsCampaigns :: Maybe MarketingCampaigns
  , saleFormOptionsDiscountElegible :: Bool
  }

instance ToJSON SaleFormOptions where
  toJSON SaleFormOptions{..} =
    Aeson.object [ "campaigns" .= saleFormOptionsCampaigns
                 , "discount_elegible" .= saleFormOptionsDiscountElegible
                 ]

getNewSaleFormOptions :: MonadHandler m => Key User -> SqlPersistT m SaleFormOptions
getNewSaleFormOptions userKey = NewTenancyFormFields <$> campaigns <*> discountElegible
 where
    campaigns =
      runMaybeT (MaybeT . fetchCampaigns userKey =<< lookupIdGetParamM "dealership")

    discountElegible =
      M.fromMaybe False <$> runMaybeT (MaybeT . fmap Just . isDiscountElegible =<< lookupIdGetParamM "vehicle")

getEditSaleFormOptions
  :: MonadHandler m
  => Key User
  -> Key Sale
  -> SqlPersistT m SaleFormOptions
getEditSaleFormOptions userKey saleKey = do
  sale <- get saleKey
  EditTenancyTenantFormFields <$> fmap join (mapM (fetchCampaigns userKey) (saleDealership <$> sale))
                              <*> (M.fromMaybe False <$> mapM isDiscountElegible (saleVehicle <$> sale))

fetchCampaigns :: MonadHandler m => Key User -> Key Dealership -> SqlPersistT m (Maybe MarketingCampaigns)
fetchCampaigns userKey dealershipKey = do
  enabled <- campaignsEnabled userKey dealershipKey
  if enabled then Just <$> getCampaigns dealershipKey
             else return Nothing

isDiscountElegible :: MonadHandler m => Key Vehicle -> SqlPersistT m Bool
isDiscountElegible vehicleKey = M.fromMaybe False <$> runMaybeT checkElegible
  where
    checkElegible = do
      vehicle <- MaybeT (get vehicleKey)
      vehicleHasDiscount <$> (MaybeT . get $ vehicleManufacturer vehicle)
                         <*> pure vehicle

lookupIdGetParamM
  :: (PathPiece (Key a), MonadHandler m)
  => Text
  -> MaybeT m (Key a)
lookupIdGetParamM = MaybeT . lookupIdGetParam

And this is the code for lookupIdGetParam:

lookupIdGetParam :: (PathPiece (Key a), MonadHandler m) => Text -> m (Maybe (Key a))
lookupIdGetParam name = do
  mraw <- lookupGetParam name
  case mraw of
    Nothing  -> return Nothing
    Just raw -> return (fromPathPiece raw)

I'd suggest these points to simplify your code:

Make everything that returns Maybe something into MaybeT, and as close to the origin as possible. This will make composition of your functions much easier. For example:

lookupIdGetParam :: (PathPiece (Key a), MonadHandler m) => Text -> MaybeT m (Key a)
lookupIdGetParam name = liftM fromPathPiece (MaybeT $ lookupGetParam name)

Notice using MaybeT just at lookupGetParam, then we don't need to duplicate MaybeT's functionality in the case expression.

This also helps you to avoid complicated code with two nested monads (such as SqlPersistT m (Maybe MarketingCampaigns)) and then complex and hard-to-read operations on them like fmap join (mapM ...).
MaybeT . fmap Just is equivalent to lift.
Whenever applicable, use functions from MonadPlus. Not just mzero (which is here equivalent to Nothing) and mplus, there are also some other very useful ones such as mfilter or guard. Like in:

fetchCampaigns :: MonadHandler m => Key User -> Key Dealership -> MaybeT (SqlPersistT m) MarketingCampaigns
fetchCampaigns userKey dealershipKey = do
  enabled <- lift $ campaignsEnabled userKey dealershipKey
  guard enabled
  lift $ getCampaigns dealershipKey

Something to try out, I'm not sure if this will help you, but it might: Instead of a function that returns a Bool in a monad, return MaybeT MyMonad () such that original False corresponds to a failure in MaybeT and True to a () value. To be specific, instead of isDiscountEligible, you could have

checkDiscountElegible :: MonadHandler m => Key Vehicle -> MaybeT (SqlPersistT m) ()
checkDiscountElegible vehicleKey = do
  vehicle <- MaybeT (get vehicleKey)
  hasDiscount <- vehicleHasDiscount <$> (MaybeT . get $ vehicleManufacturer vehicle)
                                    <*> pure vehicle
  guard hasDiscount

The idea is that this will make these functions composable with the other MaybeT ... ones, and avoid the tedious M.fromMaybe False <$> runMaybeT.

In other words, in MaybeT m Bool there are two failure modes: One failing within the monad, and another returning False. If you know from the beginning that they're both equivalent, just collapse them instead of keeping the distinction.

Disclaimer: I haven't tried to compile any of the above, so there might be syntax errors, but hopefully the meaning is clear.


Should we create a new constant or modify an existing one I was fixing a bug in the code and it was something like this:

int const ROW_LOC = 1;
//....
Foo ( x , y , ROW_LOC ) ;

so it was passing that constant to a method ... and this ROW_LOC constant is being used in multiple other places in the code too.

For ONLY ONE place in the code because this constant was getting passed to some method that was working with a zero-based index I did like this:

SomeOtherFooMethd ( x , str , ROW_LOC -1 );

So my question is: Do I need to create a separate constant with value Zero for this one method call or you would continue using the same ROW_LOC and like I did just decremneting it by 1 in this case?

It depends on the meaning of this constant. If the value ROW_LOC - 1 has logical relation to ROW_LOC (e.g. it means that it should go right before ROW_LOC in some list) then just use ROW_LOC - 1 there, but if it's just 0 - then either define a new constant or just use 0 as value, depending on situation.

What I would suggest is to rename ROW_LOC to more meaningful (descriptive) name unless it's a well-known acronym in your company.


Repository layer for MongoDB I've implemented a quick and dirty repository to save and insert users to a MongoDB. As you can see, it just inserts one and finds one. Not a rocket.

The thing is I don't really know what are the best practices when implementing a repository layer, how to really arquitect this, etc...

Furthermore, even if it sounds too simple, I'm not sure which variables should be instance properties or not (for example, the database, or the client). Assumed those where convenient, but I'm not sure on the choice.

Any tips are welcome :)

User.kt

data class User (val chatId: String, val username: String, val email: String)

BaseRepository.kt

import com.mongodb.MongoClient
import com.mongodb.MongoClientURI
import com.mongodb.client.MongoDatabase

abstract class BaseRepository<Entity>(databaseUri: String) {
    protected var database: MongoDatabase? = null
    protected var mongoClient: MongoClient? = null

    init {
        val connectionString = MongoClientURI(databaseUri)
        mongoClient = MongoClient(connectionString)
        database = mongoClient?.getDatabase("paybotDB")
    }

    abstract fun insertOne(e: Entity);

    abstract fun findOne(username: String) : Entity;
}

UserRepository.kt

import org.bson.Document
import org.jgmanzano.paybot.entities.User

class UserRepository(databaseUri: String) : BaseRepository<User>(databaseUri) {

    override fun insertOne(user: User){
        val document = Document("username", user.username)
                .append("chatId", user.chatId)
                .append("email", user.email)

        database?.getCollection("users")?.insertOne(document)
    }

    override fun findOne(username: String): User {
        val collection = database?.getCollection("users")
        val userDocument =  collection?.find(Document("username", username))?.first()

        return User(
                userDocument?.get("chatId") as String,
                userDocument?.get("username") as String,
                userDocument?.get("email") as String
        )
    }
}

Your properties are nullable even tough they are never null and assigned in the constructor. Declare them as

protected val database: MongoDatabase

This way, the child classes can access them without needing the safe-call operator and also you prevent them from overwriting the property.

Also, both of your methods call database.getCollection("users"). This is a sign that you need to extract the collection to a property rather than the database or client.

Also, it's better to pass a MongoClient or even a MongoDatabase to the constructor because it can be shared between multiple repositories and follows the dependency injection pattern.

Because you now have only one property, you can drop the init block.

abstract class BaseRepository<Entity>(db: MongoDatabase, collectionName: String) {
    protected val collection = db.getCollection(collectionName)

    abstract fun insertOne(e: Entity)

    abstract fun findOne(username: String) : Entity
}

I also got rid of the unnecessary semicolons.

Now to the child class. val userDocument =  collection?.find(Document("username", username))?.first() returns a nullable  Document?. But in the line below you cast the members to String which will throw an exception if the value was null. To get proper null safety, declare the return type of the method as nullable

abstract fun findOne(username: String) : Entity?

and rewrite the implementation, e.g. like so

override fun findOne(username: String): User? {
    val userDocument = collection.find(Document("username", username)).first() ?: return null

    return User(
            userDocument.get("chatId") as String,
            userDocument.get("username") as String,
            userDocument.get("email") as String
    )
}


Slow python code that automatically selects k for KNNG This code is used to select K for a K-nearest neighbor graph [KNNG] from a dataset with an unknown number of centroids (which is not the same as K-means clustering).

Suppose that you have a dataset of observations stored in a data matrix X[n_samples, n_features] with each row being an observation or feature vector and each column being a feature. Now suppose you want to compute the (weighted) k-Neighbors graph for points in X using sklearn.neighbors.kneighbors_graph. The first step is to pick k.

This brute force method to pick k below is very slow when the sample dataset size becomes large (ie n_neighbors_max = 10000). The FOR loop is very slow. Any tips for how to speed it up?

def autoselect_K(X, n_neighbors_max, threshold):
    # get the pairwise euclidean distance between every observation
    D = sklearn.metrics.pairwise.euclidean_distances(X, X)
    chosen_k = n_neighbors_max
    for k in range(2, n_neighbors_max):
        k_avg = []
        # loop over each row in the distance matrix
        for row in D:
            # sort the row from smallest distance to largest distance
            sorted_row = numpy.sort(row)
            # calculate the mean of the smallest k+1 distances
            k_avg.append(numpy.mean(sorted_row[0:k]))
        # find the median of the averages
        kmedian_dist = numpy.median(k_avg)
        if kmedian_dist >= threshold:
            chosen_k = k
            break
    # return the number of nearest neighbors to use
    return chosen_k

Without thinking too hard about what you are actually doing (i.e., by trying to find a considerable more efficient algorithm), you could get a decent speedup by using list comprehension. Doing appends inside an explicit for loop doesn't usually lead to high performance.
Another observation is that you are doing too much work in sorting the whole row, when you just want the first k elements in sorted order. For that, there's a more suitable function like numpy.argpartition or heapq.nsmallest.

I don't have a test program for correctness, but I wrote a simple check for performance. But even then, I have no idea about your actual use case and parameters, so take it with a grain of salt:

import numpy as np
from sklearn.metrics.pairwise import euclidean_distances
from time import time

def autoselect_K_orig(X, n_neighbors_max, threshold):
    # get the pairwise euclidean distance between every observation
    D = euclidean_distances(X, X)
    chosen_k = n_neighbors_max
    for k in range(2, n_neighbors_max):
        k_avg = []
        # loop over each row in the distance matrix
        for row in D:
            # sort the row from smallest distance to largest distance
            sorted_row = np.sort(row)
            # calculate the mean of the smallest k+1 distances
            k_avg.append(np.mean(sorted_row[0:k]))
        # find the median of the averages
        kmedian_dist = np.median(k_avg)
        if kmedian_dist >= threshold:
            chosen_k = k
            break
    # return the number of nearest neighbors to use
    return chosen_k

def autoselect_K_other(X, n_neighbors_max, threshold):
    D = euclidean_distances(X, X)
    for k in range(2, n_neighbors_max):
        kmedian_dist = np.median([np.mean(row[np.argpartition(row, k)]) for row in D])
        if kmedian_dist >= threshold:
            return k

    return n_neighbors_max

# Rudimentary performance check
X = np.random.randint(5, size=(10000, 10))

start = time()
result = autoselect_K_orig(X, 20, 4)
end = time()
total_orig = end - start
print('Original: ', total_orig)

start = time()
result = autoselect_K_other(X, 20, 4)
end = time()
total_other = end - start
print('Other: ', total_other)

print('Speedup: ', total_orig / total_other)

On my outdated hardware, a typical output is:

Original:  57.56129217147827
Other:  2.257129192352295
Speedup: 25.501992693422242

Meaning speedups of more than 25x. Not surprisingly, this speedup is strongly linked to the value of n_neighbors_max; if you make it very small, the speedup tends to go as low as 2x, but as you can see, it grows quite considerably for larger values, which is probably exactly what you want.

I think further optimization even with your current approach is still possible. For instance, notice that everytime that you increment k, you are essentially duplicating work you have already done by again taking the first (k-1) elements, taking their mean, and so on. Instead, for step k, you should have stored the mean of step (k-1), and then you simply update this mean with a new element that now comes in, reducing the amount of computation you need to do.
Even without such an optimization, you could still get some gains by getting rid of the explicit for loop that goes from 2 to n_neighbors_max by using a suitable generator expression that asks for more elements for as long as we've not extracted an element of at least the threshold.


Powerset of collection in Clojure Similar to this question, here's another implementation. Assume the input is already a set.

 (defn powerset
       [set]
       (reduce
           (fn [xs x] (concat xs (map #(cons x %) xs)))
           [()]
           set))

There isn't a whole lot here to comment on. I'll just mention a few things:

Technically, from my quick search of what a powerset is, this function should return sets. That seems petty, but unless it's documented to return a lazy list of lazy lists, users may try to treat the "subsets" as sets (like using them as functions). I'd finish this function off by mapping set over the list.
But to do that, you should rename your parameter, as you're shadowing the build-in set.
After doing the above two, it developed quite long lines and became nested. I'd add in some use of ->>, and put a few of the lines on the next line.

After that, I ended up with:

(defn powerset [base-set]
  (->> base-set
       (reduce
         (fn [xs x]
           (concat xs
                   (map #(cons x %) xs)))
         [()])

       (map set)))


Turning Conduits into Pipes Does this look right to you? What are its shortcomings? Strengths?

import Control.Monad
import Control.Monad.Trans.Class
import Control.Pipe.Common
import Data.Conduit

c2p :: (Resource m, Monad m) => Conduit a m b -> Pipe a b m ()
c2p = do
  PreparedConduit push close <- lift $ runResourceT $ prepareConduit c
  loop push close
  where loop push close = do
          input <- await
          stepResult <- lift $ runResourceT $ push input
          case stepResult of
            Producing output -> do
              mapM_ yield output
              loop push close
            Finished _ output -> do
              mapM_ yield output
              lift $ runResourceT $ close
              return ()

I can see three problems with this approach:

1) Calling runResourceT at each step releases all the scarse resources allocated by the original conduit. This is not what you want, since a conduit can depend on the availability of those resources until the point where it terminates.

2) You cannot call close after push returns Finished. This is explicitly stated in the documentation of PreparedConduit.

3) You need to call close on the conduit when the upstream pipe terminates. Unfortunately, I think this is impossible to do with vanilla pipes. That's why I am currently experimenting with what I call "guarded pipes". You can find them here, together with some other utilities ported over from conduits.

Using guarded pipes, you can write something like:

c2p :: Resource m => Conduit a m b -> Pipe a b (ResourceT m) ()
c2p c = do
  PreparedConduit push close <- lift $ prepareConduit c
  loop push close
  where
    loop push close = do
      input <- tryAwait
      case input of
        Nothing -> do
          output <- lift $ close
          mapM_ yield output
        Just input -> do
          stepResult <- lift $ push input
          case stepResult of
            Producing output -> do
              mapM_ yield output
              loop push close
            Finished _ output ->
              mapM_ yield output

You can try it, for example, with:

main = runResourceT . runPipe $
  fileProducer "sample.txt.gz" >+> c2p CZ.ungzip >+> fileConsumer "sample.txt"

where fileProducer and fileConsumer are also contained in the above pipes-extra package.


Filtering numeric vectors by proximity to any elements of another vector in R I frequently want to filter and select elements from a long (10,000s to millions of elements) vector in R.  Often, this vector is a column in a data.frame.  I am usually interested in rows of the vector that "match" any one of a separate list of 10 to 100 query values.  However, since these are experimental data (mass spectrometry data for anyone interested), the target values and the query value(s) do not need to match exactly.  They only need to match within a tolerance.

In the field I work in (mass spectrometry), the values are mass-to-charge ratios, or m/z values.  There are two systems for specifying the tolerance or precision desired for comparing two numbers.  The first is the ppm or "parts per million" tolerance, i.e., a relative measure of precision.  The second is in absolute values, which invariably have units of atomic mass units or Daltons.  This explains the variable names I use below.

filter_by_mz <- function(query_mzs, target_mzs, ppm = 10, mz_tol = NULL){
    # ensure that users only supply one mass tolerance argument (either ppm or mz_tol)
    if(!missing(ppm) & !missing(mz_tol)){
        stop("Only a single mz tolerance parameter {mz_tol or ppm} can be supplied, not both.")
    }

    # find min and max mzs
    if(missing(mz_tol)){
        min_mzs <- target_mzs * (1 - ppm / 1e6)
        max_mzs <- target_mzs * (1 + ppm / 1e6)
    } else{
        min_mzs <- target_mzs - mz_tol
        max_mzs <- target_mzs + mz_tol
    }

    # allocate boolean matrix
    n_rows <- length(query_mzs)
    n_cols <- length(target_mzs)
    bool_mat <- matrix(FALSE, nrow = n_rows, ncol = n_cols)

    # loop over target mzs to create columns of bool_mat
    for(idx in seq_along(target_mzs)){
        min_mz <- min_mzs[idx]
        max_mz <- max_mzs[idx]
        bool_mat[, idx] <- query_mzs >= min_mz & query_mzs <= max_mz
    }

    # apply any() to rows of matrix
    return(apply(bool_mat, FUN = any, MARGIN = 1))
}

Now my questions:

How can I get rid of the for loop?  It seems like a smart way of using apply or a similar function would obviate this loop.
Is the argument handling OK?  I wanted users of the function to be able supply either ppm or mz_tol, but not both.  Am I using missing() correctly?
Is the design correct?  As I mentioned, I frequently will invoke this function on the columns of a data.frame.  For example, I'd frequently need to do my_filtered_df <- filter_by_mz(my_df$mz, target_mz, ...).  Would it be better to write the function to work on data.frames rather than just vectors?

I do quite similar stuff.   I find your approach quite lengthy.  Especially the construction of the boolean matrix might be a bottleneck.

First I would consider to write a second function - say filter_by_mz_ which just does the filtering and have filter_by_mz just take care of the input and call the new function - just to keep things modular.

In your algorithm you basically compare all masses pairwise.  A simpler approach might be to use expand.grid(q=query_mzs, t=target_mzs) this will calculate all pairs.  This list can be then easy checked (let's call it gr) using abs(gr$t-gr$q) < ppm * gr$t - that would be fully vectorized.

A bool vector can usually be easy constructed using %in%.

Just FYI  I find it much easier to write with dplyr - especially since it is possible to keep much additional info in the table.  I often have the constraint that one mz can only be assigned once - extra data is often required to make such decisions.

Typically it looks like this:

Just the setup and some test data:

library(dplyr)

peaks <- data.frame(mz=c(21.01,28.00,30,30.1,35, 21.02), val=c(1,2,3,4,5,6))
info <- data.frame(mz=c(21.008, 28.003, 30.002, 32.02), str=c('wat', 'n2', 'aa' , 'bb'))

The magic happens here:

keyfun <- function(x) sprintf('%.1f',x)
peaks <- mutate(peaks, key=keyfun(mz))
info <- mutate(info, key=keyfun(mz))

inner_join(peaks, info, by='key') %>% filter( abs(mz.x - mz.y) < 0.0005 * mz.x)

Basically I create an temporary key that is too wide - just that I only get to work on combinations that could be.  The inner_join will create all the permutations.  And in a second step I filter out the ones that actually match.

Of course usually I give more informative names and spend some time labelling stuff.  But this should suffice to give an idea of the method.

addition

It came to me that you could avoid the matrix if you could rewrite such that inside the loop you check for all query_mz instead of target_mz - then you could also put the any inside the loop.  I suppose it would come down to just switching the variable names in a number of lines.
After this transformation the for loop should be replacable by an sapply.

As R stores logicals quite expensive with 4 bytes per logical. That shoud give you extensive savings. (For me these numbers would go into the 100s so I would have >99% less memory usage)


Adapter class displaying characters' info I have finished creating an adapter class which displays characters' info.
When I looked back and reviewed my own code, I think the code fragment below somehow could be broken into smaller methods.

...
private static class ViewHolder {
    ImageView imgIconState;
    ImageView imgCharacterStatus;
    TextView txtName;
}

@Override
public View getView(int index, View view, ViewGroup viewGroup) {
    ViewHolder characterProfile;
    if (view == null) {
        LayoutInflater inflater = (LayoutInflater) mContext.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        view = inflater.inflate(R.layout.character_item, null);
        characterProfile = new ViewHolder();
        characterProfile.imgIconState = (ImageView) view.findViewById(R.id.icon_state);
        characterProfile.imgCharacterStatus = (ImageView) view.findViewById(R.id.character_state);
        characterProfile.txtName = (TextView) view.findViewById(R.id.name);
        view.setTag(characterProfile);
    } else {
        characterProfile = (ViewHolder) view.getTag();
    }

    Character character =  mLeBluetoothLightsList.get(index);
    characterProfile.imgIconState.setImageResource(mIconStateRrs[character.getIconStatus()]);
    characterProfile.imgCharacterStatus.setImageResource(character.isAlive() ? R.drawable.alive : R.drawable.dead);
    characterProfile.txtName.setText(character.getCharacterName());

    if (Config.IS_DEBUG) {
        Log.i(mClassName, "getView - Index: " + index
                + "| Icon status: " + character.getIconStatusForLog()
                + "| Character status: " + character.isAliveForLog()
                + "| Name: " + character.getCharacterName());
    }

    return view;
}
...

I am attempting to write a 3 to 10 lines function, but I find it hard to break it smaller.

Should I refactor this or leave it as it is? If the answer is the former, please give me some advice.

I think this method is not that long that you really need to refactor it, but you could do something like

public View getView(int index, View view, ViewGroup viewGroup) {
    view = createViewIfNeeded(view)
    final ViewHolder characterProfile = (ViewHolder) view.getTag();
    Character character =  mLeBluetoothLightsList.get(index);
    characterProfile.imgIconState.setImageResource(mIconStateRrs[character.getIconStatus()]);
    characterProfile.imgCharacterStatus.setImageResource(character.isAlive() ? R.drawable.alive : R.drawable.dead);
    characterProfile.txtName.setText(character.getCharacterName());
    logIfDebug(index, character);
    return view;
}

private static void logIfDebug(int index, Character character){
    if (Config.IS_DEBUG) {
        Log.i(mClassName, "getView - Index: " + index
                + "| Icon status: " + character.getIconStatusForLog()
                + "| Character status: " + character.isAliveForLog()
                + "| Name: " + character.getCharacterName());
    }
}

// You could extract createAndInitCharacterProfile from this method if you want
private View createViewIfNeeded(View view){
    if (view == null) {
        LayoutInflater inflater = (LayoutInflater) mContext.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        view = inflater.inflate(R.layout.character_item, null);
        ViewHolder characterProfile = new ViewHolder();
        characterProfile.imgIconState = (ImageView) view.findViewById(R.id.icon_state);
        characterProfile.imgCharacterStatus = (ImageView) view.findViewById(R.id.character_state);
        characterProfile.txtName = (TextView) view.findViewById(R.id.name);
        view.setTag(characterProfile);
   }
   return view;
}


MIPS assembly addition program I am new to assembly and have made a simple addition program to sum two integers read from the keyboard. The program outputs correctly, but I want to know if there is a way to streamline my code. It seems a bit cumbersome for such a simple program and I may have instructions that are unnecessary.

# Author: Evan Bechtol
# Description: This program prompts the user to enter 2 integers and computes their sum.
#---------------------------------------------------------------------------------------#
        .data
A:          .word       # Store the number 4 as an integer in var1  # $t0 is used
B:          .word       # Store the number 2 as an integer in var2  # $t1 is used
S:          .word       # Store the sum of A and B          # $t2 is used
Prompt1:    .asciiz "Please enter first number: "
Prompt2:    .asciiz "Please enter second number: "
Result:     .asciiz "The sum of A and B is: "
        .text
main:
    #--------------------------------------------------------#
    #Display first prompt
    li  $v0, 4      # Load instruction "print string"
    la  $a0, Prompt1    # Load prompt into $a0
    syscall

    #Read first integer
    li  $v0, 5      # Read 1st integer
    la  $t0, A      # $t0 = A
    syscall

    #Store first integer into memory
    move    $t0, $v0    # Move contents in $v0 to $t0
    sw  $t0, A      # A = value at $t0
    #--------------------------------------------------------#

    #Display second prompt
    li  $v0, 4      # Load instruction "print string"
    la  $a0, Prompt2    # Load prompt into $a0
    syscall

    #Read second integer
    li  $v0, 5      # Read 1st integer
    la  $t1, B      # $t0 = A
    syscall

    #Store second integer into memory
    move    $t1, $v0    # Move contents in $v0 to $t0
    sw  $t1, B      # A = value at $t0
    #--------------------------------------------------------#

    #Add the two variables
    la  $t2, S      # $t2 = S
    add     $t2, $t0, $t1   # $t2 = $t0 + $t1
    sw  $t2, S      # S = value at $t2

    #Display the Result prompt
    la  $a0, Result # Loads Output label to be printed
    li  $v0, 4      # Sysycall to print string
    syscall

    #Display the sum
    lw  $a0, S      # $a0 = value at S
    li  $v0, 1      # Syscall to print integer
    syscall

    #Exit the program
    li  $v0, 10     # Load exit code to $v0
    syscall

The comments are misleading:

#Read second integer
li  $v0, 5      # Read 1st integer
la  $t1, B      # $t0 = A

umm... are we reading second or 1st? Bottomline is, do not overcomment the code.
syscall 5 leaves a value in $v0. The contents of $t0 (or $t1) is irrelevant during the syscall. Set them up when you need them:

li $v0, 5
syscall
la $t0, A
move    $t0, $v0

You store data to memory just to load them back. This is very anti-assembly. Generally you want to use registers as much as possible, and avoid memory as much as possible:

li $v0, 5
syscall
move $t0, $v0
...
li $v0, 5
syscall
# At this moment you have first integer in $t0, and the second in $v0.
# Just add them together. No memory access is necessary.

Consult your documentation on which registers are guaranteed to survive a syscall (I suspect, all of them besides $v0).
Nothing to simplify reading and printing.


Homebrew std::string for use with kernel I've ported some standard library facilities like vector, algorithm, etc. to my kernel so I can code in C++ instead of C. This is one of them. I'm looking for concerns regarding:

Performance
Safety
Friendly interface

Perhaps there are more idiomatic ways to accomplish what I'm going for?

#ifndef _LIBCPP_STRING_H_
#define _LIBCPP_STRING_H_

#include <prefix.h>
#include <libc/stddef.h>
#include <libc/string.h>
#include <libc/stdint.h>

#include "algorithm.h"

namespace std
{
    template<size_t Size>
    class fixed_string
    {
    public:
        static constexpr size_t npos = static_cast<size_t>(UINT32_MAX);

        fixed_string() :
            _length(0)
        {
            _buffer[0] = '\0';
        }
        fixed_string(const char *string)
        {
            assign(string, strlen(string));
        }
        fixed_string(const fixed_string &other)
        {
            assign(other._buffer, other._length);
        }

        fixed_string &operator =(const fixed_string &other)
        {
            assign(other._buffer, other._length);
            return *this;
        }
        fixed_string &operator =(const char *string)
        {
            assign(string, strlen(string));
            return *this;
        }

        bool operator ==(const fixed_string &other) const
        {
            if(_length != other._length)
                return false;

            return (strcmp(other._buffer, _buffer) == 0);
        }
        bool operator ==(const char *string) const
        {
            return (strcmp(string, _buffer) == 0);
        }

        bool operator !=(const fixed_string &other) const
        {
            return !(*this == other);
        }
        bool operator !=(const char *string) const
        {
            return !(*this == string);
        }

        size_t find(const fixed_string &other) const
        {
            return find(other._buffer, _length);
        }
        size_t find(const char *other) const
        {
            return find(other, strlen(other));
        }
        size_t find(const char *other, size_t length) const
        {
            size_t found  = 0;

            char *buffer = _buffer;
            while(*buffer)
            {
                if(found >= length)
                {
                    size_t diff = ((buffer - found) - _buffer);
                    return diff;
                }

                found = (*buffer == other[found]) ? found + 1 : 0;
                buffer ++;
            }

            return npos;
        }

        const char *c_str() const { return _buffer; }
        size_t length() const { return _length; }

    private:
        void assign(const char *string, size_t length)
        {
            if(!string)
            {
                _buffer[0] = '\0';
                _length = 0;

                return;
            }

            _length = std::min(length, Size);
            strlcpy(_buffer, string, _length);
        }

        char _buffer[Size + 1];
        size_t _length;
    };
}

#endif /* _LIBCPP_STRING_H_ */

In operator=(), you should always first check to see whether you're doing a self-assignment and do an early exit:

fixed_string &operator =(const fixed_string &other)
{
    if (this == &other) {
        return *this;
    }
    assign(other._buffer, other._length);
    return *this;
}

I would write the found assignment as the more explicit:

if (*buffer == other[found]) {
    found++;
} else {
    found = 0;
}

The compiler will generate the equivalent code to yours.

In fact, your find has a bug. Consider:

fixed_string<10> a = "abcabcd";
assert(a.find("abcd") == 3);

Your assign method calls strlcpy incorrectly. Instead of calling with _length as the third parameter, use the actual size of the buffer:

strlcpy(_buffer, string, sizeof(_buffer));

As a matter of style, I would always surround conditional blocks with braces, even if they only contain one statement:

bool operator ==(const fixed_string &other) const
{
    if(_length != other._length) {
        return false;
    }
    // ...


Incrementing Values inside an if statement I found out that I can increase the value of a variable inside an if statement like so:
int limit = 100;
int nrCopies = 2;
int maxValue = 100;
for(int i = 0; i < limit; ++i)
{
    if((nTotal += (nrCopies)) > maxValue)
        break;
}

This works, and I became acostumed to do this, but I want to know if it is a good practice to do so, since I don't want to get bad habits unnecessarily

I would say this is quite questionable.
Imagine you never saw this code before, and are trying to understand it, would this make it easier to understand or harder? I say harder.


Suggestion for improving this code to be more readable and easy for beginners? I'm a JavaScript beginner trying to learn by doing. Here I have a table with different values: I have used two for loops. Is there an easier way to achieve this? For example having just one for loop, or instead of it use something else? Even better would be if you could give suggestions to improve the code to be more readable (beginner friendly?)

let animals
let animalCols = ['Animal', 'Animal 2']
let peopleCols = ['Person', 'Person 2']

function myFunction() {
  paivitys(animals, animalCols)
}

function paivitys(dataa, arvvoja) {
  console.log(dataa);
  //----
  if (dataa.hasOwnProperty("animal")) {
    document.getElementById("1name").innerHTML = dataa.animal;
  } else {
    document.getElementById("1name").innerHTML = dataa.person;
  }
  //----
  if (dataa.hasOwnProperty("animal2")) {
    document.getElementById("2name").innerHTML = dataa.animal2;
  } else {
    document.getElementById("2name").innerHTML = dataa.person2;

  }

  document.getElementById("1name1").innerHTML = arvvoja[0];
  document.getElementById("2name1").innerHTML = arvvoja[1];

  //-----
  document.getElementById("id").innerHTML = dataa.id;
}

function paivitaselekt(dataa, arvvoja) {
  for (var i = 0; i < dataa.length; i++) {
    var valitse = document.getElementById("Select");
    var option = document.createElement("option");

   for (var i = 0; i < dataa.length; i++) {
    var valitse = document.getElementById("Select");
    var option = document.createElement("option");
    for (var j = 0; j < arvvoja.length; j++) {
       option.textContent += dataa[i][arvvoja[j]] + " ";
    }
    valitse.appendChild(option);
} ;
    valitse.appendChild(option);
  }
}

animals = {
  "animal": "tiger",
  "animal2": "lion",
  "id": "54321",
  "dole": {
    "Key": "fhd699f"
  }
}

paivitys(animals, animalCols);
let kokoarray;

people = [{
    "person": "kaka",
    "person2": "julle",
    "id": "9874",
  },
  {
    "person": "Ronaldo",
    "person2": "jussi",
    "id": "65555",
  }
]
kokoarray = people;
paivitaselekt(kokoarray, ["person", "id"]);

document.getElementById("Select").addEventListener("change", function(event) {
  const otettutunnsite = event.target.value.split(" ")[1];
  const otettutieto = kokoarray.filter((dataa) => dataa.id === otettutunnsite)[0];
  paivitys(otettutieto, peopleCols);
});
<!DOCTYPE html>

    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width" />
    <link
      rel="stylesheet"
      href="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css"
    />
    <link
      rel="stylesheet"
      href="https://use.fontawesome.com/releases/v5.6.3/css/all.css"
      integrity="sha384UHRtZLI+pbxtHCWp1t77Bi1L4ZtiqrqD80Kn4Z8NTSRyMA2Fd33n5dQ8lWUE00s/"
      crossorigin="anonymous"
    />

        Animal
        Animal
        id

  <select id="Select" ></select>
  <button onclick="myFunction()">backtozero</button>

Some suggestions:

Since you never reassign animals, better to declare it with const. (On a similar note, don't mix let and var - var has too many gotchas, and let permits reassignment, which is more difficult for a reader of the code to parse than const. Always prefer const.)

You never declare the people variable. This will throw an error in strict mode, and in sloppy mode, will implicitly create a variable on the global object, even if the whole script is inside its own function. Best to always declare variables before using them. Since the kokoarray is the same as people, it'll be easier to just use one of those variables, no need for two identical ones which refer to the same array.

Regarding your myFunction, don't use inline handlers, they have way too many problems to be worth using in modern scripts; use addEventListener instead:

document.querySelector('button').addEventListener('click', () => {
  paivitys(animals, animalCols)
});

Rather than selecting elements over and over, it would be more elegant to select the elements you need once, then reference those variables rather than calling querySelector again. It's less repetitive and less expensive.

Numeric-indexed IDs are a code smell. IDs should be reserved for elements which are completely unique in the document. If you want to indicate that an element fulfills a particular role that other elements have as well, use a class or a data attribute instead.

Your
 is missing a child
 - both s and s should only be children of
s. Also, s should be reserved for table headers. For plain data (like in your body), it'd be more semantically appropriate to use tds. If you want to make them bold, use CSS for that.

Rather than having animal and animal2 and person and person2, it'd be a lot simpler to combine all of those properties into a single array property, perhaps call it names. Rather than having multiple entirely separate data structures, it'll be far easier to parse it if they have the same format. One option is:

const animalData = {
  cols: ['Animal', 'Animal 2'],
  items: [{
    names: ['tiger', 'lion'],
    id: "54321",
  }]
};
const people = {
  cols: ['Person', 'Person 2'],
  items: [{
      names: ['kaka', 'julle'],
      id: "9874",
    },
    {
      names: ['Ronaldo', 'jussi'],
      id: "65555",
    }
  ]
};

This way, to reference the first (only) animal, use animalData.items[0]. To reference the ith person, reference people.items[i]. Populating the table gets much easier now:

const headTR = document.querySelector('thead tr');
const bodyTR = document.querySelector('tbody tr');
function display(cols, item) {
  headTR.children[0].textContent = cols[0];
  headTR.children[1].textContent = cols[1];
  bodyTR.children[0].textContent = item.names[0];
  bodyTR.children[1].textContent = item.names[1];
  bodyTR.children[2].textContent = item.id;
}

Remember to use textContent when inserting text - using innerHTML when you're not deliberately inserting HTML markup makes things slower and can be a security hazard if the input isn't trustworthy.

To identify which person item should be displayed when the select changes, you can set the value of each <option> to its current index, eg:

<select>
  <option value="0">kaka 9874</option>
  <option value="1">Ronaldo 65555</option>
</select>

Then, when the select changes, just pass people.items[select.value] to the function that populates the table, rather than having to .find.

You can construct the options much more concisely, like the following:

people.items.forEach((item, i) => {
  const option = select.appendChild(document.createElement('option'));
  option.textContent = `${item.names[0]} ${item.id}`;
  option.value = i;
});

Put all these together, and you get:

const animalData = {
  cols: ['Animal', 'Animal 2'],
  items: [{
    names: ['tiger', 'lion'],
    id: "54321",
  }]
};
const people = {
  cols: ['Person', 'Person 2'],
  items: [{
      names: ['kaka', 'julle'],
      id: "9874",
    },
    {
      names: ['Ronaldo', 'jussi'],
      id: "65555",
    }
  ]
};

const headTR = document.querySelector('thead tr');
const bodyTR = document.querySelector('tbody tr');
function display(cols, item) {
  headTR.children[0].textContent = cols[0];
  headTR.children[1].textContent = cols[1];
  bodyTR.children[0].textContent = item.names[0];
  bodyTR.children[1].textContent = item.names[1];
  bodyTR.children[2].textContent = item.id;
}

display(animalData.cols, animalData.items[0]);
document.querySelector('button').addEventListener('click', () => {
  display(animalData.cols, animalData.items[0])
});

const select = document.querySelector("select");
select.addEventListener("change", function(event) {
  display(people.cols, people.items[select.value]);
});
people.items.forEach((item, i) => {
  const option = select.appendChild(document.createElement('option'));
  option.textContent = `${item.names[0]} ${item.id}`;
  option.value = i;
});
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css">
<link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.6.3/css/all.css" integrity="sha384UHRtZLI+pbxtHCWp1t77Bi1L4ZtiqrqD80Kn4Z8NTSRyMA2Fd33n5dQ8lWUE00s/" crossorigin="anonymous">

        Animal
        Animal
        id

  <select></select>
  <button>backtozero</button>


Making toString function more readable In Java I'm trying to make my code more readable. I created a function toString - It returns a string in the following format:

Name: <Name>.
IdNumber: <Number>.
Age: <Number>.
Colors: <Color1, Color2, ..., ColorN>.

Please notice that the colors should be sorted alphabetically. The function I wrote for now:

public String toString() {
    String colorsString = this.colors.stream().sorted().collect(Collectors.joining(", "));
    return "Name: " + this.Name+".\n" +
            "IdNumber: "  + this.Id+".\n" +
            "Age: " + this.Age+".\n" +
            "Colors: " + colorsString + ".";
}

I don't like the way I sorted the colors. I would like to use stream, but in other way if possible. Also, I feel like the format should be constant. Is it possible to keep it as a constant and add values to it? Future more I don't like the use of \n - is it possible without it?

This answer to a question on multi line strings contains a lot of good examples of trouble with building multi line string output this way but in particular I think the String.format approach works best with a template string.

Your setup would be something like

String template = “ Name: %s %n ID: ...”

And you would place it with

String formatted = String.format(template, nameString, idString,...);

Don’t like their sorting algorithm? I’d recommend extending your own! You can provide your own comparator for the sort stream function. This gives you complete control over how it sorts, at the expense of providing the desired sorting logic.
This article contains several examples of ways to extend your own comparators to this function


Imposing a sort order on a query I've always been told "never use goto, there's always a better way" and for the longest time I just accepted it. Lately though, I've been running into such scenarios in which I have to repeat this bit of code every time I return early.

I personally hate repeating myself, even the littlest bit of code. So I ask, Is this use of goto really that bad? It prevents me from having to write that bit of code 3 times and sure, theoretically I could write some guard clauses (that could be combined into a single guard clause) that prevents the need for the try/catch and therefore having a single place for that bit of code, but would that actually be more readable?

public static IQueryable<T> Sort<T>(this IQueryable<T> query, String field, String direction, Expression<Func<T, Int32?>> defaultSort = null)
{
    if (String.IsNullOrWhiteSpace(field)) { goto FAILED; }

    try
    {
       // Dynamic LINQ - Field: "SomeProperty.OptionalProperty", Direction: "Desc" or "Asc"
        query = query.OrderBy(String.Format("{0} {1}", field, direction));
    }
    // NOT FOUND: Field wasn't found just return the original query.
    catch (NullReferenceException)
    {
        goto FAILED;
    }
    // NOT FOUND: Field wasn't found just return the original query.
    catch (ParseException)
    {
        goto FAILED;
    }

    return query;

    // SOMETHING FAILED: Return the original query with the optionally supplied default sort.
    FAILED:
        if (defaultSort != null)
        {
            query = query.OrderBy(defaultSort);
        }
        return query;
}

I'm not a member of the “goto is evil and must not be used under any circumstances” camp.  Especially in performance-critical low-level routines, it can be useful from time to time.  However, most of the time, the temptation to use goto actually stems from missing a better opportunity to structure the code.  In your example, I'd simply re-structure it like this, which I find is not only more readable but also much shorter.

public static IQueryable<T>
Sort<T>(this IQueryable<T> query,
        String field,
        String direction,
        Expression<Func<T, Int32?>> defaultSort = null)
{
    if (!String.IsNullOrWhiteSpace(field)) {
        try {
            return query.OrderBy(String.Format("{0} {1}", field, direction));
        } catch (NullReferenceException) {
            // NOT FOUND: Field wasn't found just return the original query.
        } catch (ParseException) {
            // NOT FOUND: Field wasn't found just return the original query.
        }
    }
    if (defaultSort != null) {
        query = query.OrderBy(defaultSort);
    }
    return query;
}


Implement a Finite State Machine in ES6 I have built a simple finite state machine that models an even simpler fictitious bank account system.

Any given account can be in one of three states at any time: Open, Closed, or Held.

I have a few different methods that transition from one state to another:

reopen: closed -> open
close : open / held -> closed
place_hold  : open -> held
remove_hold : held -> open

I have used an ES6 class and prototypes to implements this. three different objects, each with its own methods, are set to be the __proto__ of any given BankAccount object.

The problem with this is that changing the __proto__ of an object removes its ability to call its class's methods, which is a problem.

Here is a stripped down version of my code:

const open ={stateName:"Open",
  close: function(){
    this.changeState(closed);
  },
  reopen: function(){
    return;
  },
  place_hold: function(){
    this.changeState(held);
  },
  remove_hold: function(){
    return;
  }
}

const held ={stateName:"Held",
  close: function(){
    this.changeState(closed);
  },
  reopen: function(){
    return;
  },
  place_hold: function(){
    return;
  },
  remove_hold: function(){
    this.changeState(open);
  }
}

const closed ={stateName:"Closed",
  close: function(){
    return;
  },
  reopen: function(){
    this.changeState(open);
  },
  place_hold: function(){
    return;
  },
  remove_hold: function(){
    return;
  }
}

class BankAccount {
  constructor() {
    this.balance = 0;
    this.__proto__ = open;
    this.changeState = state=>this.__proto__ = state;
  }
  otherStuff(){
    // this cannot be called...
  }
}

What would be a more proper way to do this, keeping our accounts able to access the class's methods, as well as the syntax used to access the methods implemented through the state machine?

Object properties.

If you overwrite the prototype you will lose existing properties.

There are other solutions using the class syntax, but it is important to note the following.

  JavaScript classes, introduced in ECMAScript 2015, are primarily
  syntactical sugar over JavaScript's existing prototype-based
  inheritance. The class syntax does not introduce a new object-oriented
  inheritance model to JavaScript.

from MDN Classes

Using class has some major problems, and in my opinion should be avoided as the same and better can be done using standard syntax for objects.

A word on code.

You don't need to add return to a function. Its automatic.
Use function shorthand when adding functions to objects.
Don't mix naming styles. You have snake_case and camelCase.

Apply that to

const held ={stateName:"Held",
  close: function(){
    this.changeState(closed);
  },
  reopen: function(){
    return;
  },
  place_hold: function(){
    return;
  },
  remove_hold: function(){
    this.changeState(open);
  }
}

Becomes

const held = {
  stateName: "Held",
  close() {              // function shorthand
    this.changeState(closed);
  },
  reopen() {},           // return implied Same as return undefined;
  placeHold() {},        // camelCase
  removeHold() {
    this.changeState(open);
  }
};  // objects have semicolons..

Object properties.

When you are creating objects with dynamic properties you can assign properties directly to the object using Object.assign(obj, properties) This has some performance benefits as well.

The following is an example of an instanceable account state machine using object assign to assign the the state.

const Account = (() => {
    const actions = {
        close() { this.transition(states.closed) },
        hold()  { this.transition(states.held) },
        open()  { this.transition(states.open) },
    };
    const nulls = {close(){ }, hold(){ }, open(){ }};
    const states = {
        open : {
            state: "Open",
            close: actions.close,
            hold: actions.hold,
        },
        held : {
            state: "Held",
            close: actions.close,
            open: actions.open,
        },
        closed : {
            state: "Closed",
            open: actions.open,
        }
    };
    return function(){
        const account = {
            ...states.closed,  // default State
            balance : 0,
            transition(state) { Object.assign(account, nulls, state) },
            statement() { console.log(account) },
        };
        return account;
    };
})();

To use

const foo = Account();
// or
const foo = new Account();

foo.close();
foo.open();
foo.hold();
foo.statement();


Taskqueue multithreading I have a synchronized list that I use as a taskqueue. In this queue I have some long values that need to be proceed and create heavy I/O and delay.  I thought of using threads to get things done faster.

Is there a better way to implement all 50 threads to wait than using t.join();?  Is there anything wrong with the way I access the queue in my run() implementation?

Implementation:

public class Start {

    private static List<Long> queue = Collections.synchronizedList(new ArrayList<Long>());
    private static HashSet<MyThread> threads = new HashSet<MyThread>();

    public static void main(String[] args) throws InterruptedException {
        // add elements to workqueue
        for(int i = 0; i < 100000; i++) {
            queue.add(new Date().getTime());
        }

        // create 50 new threads
        for (int i = 0; i < 50; i++) {
             MyThread t = new MyThread(queue);
             t.start();
             threads.add(t);
        }

        // wait for all 50 threads to finish
        for (MyThread t : threads) {
            t.join();
        }

        System.out.println("Done");
    }
}

Thread:

public class MyThread extends Thread {
    private List<Long> queue = null;

    public MyThread(List<Long> queue) {
        this.queue = queue;
    }

    @Override
    public void run() {
        boolean _break = false;
        int size = 0;
        long task = -1;
        while(true) {
            synchronized (queue) {
                if(!queue.isEmpty()) {
                    size = queue.size();
                    // get task for worker
                    task = queue.remove(0);
                } else {
                    _break = true;
                }
            }
            if(_break == true) {
                System.out.println("Thread-" + this.getId() + " Size: 0");
                break;
            } else {
                // do some stuff with the task
                // ...
                // ...
                System.out.println("Thread-" + this.getId() + " Size: " + size);
            }
        }
    }
}

Use an Executor
Maintaining a work queue and a thread pool are problems that an executor tackles:

This interface provides a way of decoupling task submission from the mechanics of how each task will be run, including details of thread use, scheduling, etc. An Executor is normally used instead of explicitly creating threads.

The use of Futures also helps you impose time constraints and handle exceptions thrown during execution.
(Note that you'll typically want to use ExecutorService or CompletionService as Executor is a minimalistic interface.)
To paraphrase your example:
class IncrediblyLongRunningTask implements Callable<Long> {
  @Override
  public Long call() throws Exception {
    // ...
  }
}

public void compute() throws InterruptedException {
  final ExecutorService service = Executors.newFixedThreadPool(50);
  final List<IncrediblyLongRunningTask> tasks = new ArrayList<>();
  for ( int i = 0; i < /*...*/; i++ ) {
    task.add(new IncrediblyLongRunningTask());
  }
  final List<Future<Long>> futures = service.invokeAll(tasks); // returns when all done
  service.shutdown();
  for ( final Future<Long> future : futures ) {
    // ...
  }
}


Calculate amount of elements in collection by property I was given this task: Calculate amount of elements in collection by property: "name"

 let array = [
    {name: 'Serhii'},
    {name: 'Serhii'},
    {name: 'Nile'},
    {name: 'Apple'},
    {name: 'Serhii'},
    {name: 'Serhii'},
    {name: 'Apple'},
    {name: 'Serhii'},
];

Here is my solution:

 function calculateElements(property, arr) {
    let amount = {};

    arr.forEach((e) => {
        let p = e[property];
        p in amount ? amount[p]++ : amount[p] = 1;
    });
    return amount;
}
console.log(calculateElements('name', array));

Is there more efficient way to do it?

That looks quite fine to me. You can remove the parentheses around e in the arrow function.

If you're really going for efficiency, a regular for loop is faster than the .forEach() method. That can help if you have a very large amount of objects to process and need to display a result instantly to a user, but otherwise the optimization is unnecessary for what you're trying to accomplish.


I created a "enum string" container in c++ The reason I created this is because I wanted my string values to be type safe and I wanted to decrease the amount of bad data that has crept into my system due to hard coded values in the past.  There are different ways to solve this issue, like a lookup table with a key value pair but this is not a direction I can move into at the time.  So here is my design and I would love some feedback.

Here is my working sample: https://onlinegdb.com/rympFKEaf

#include <iostream>

class EnumStringContainer
{
protected:
  EnumStringContainer(const std::string &_str);

public:
  bool operator==(const EnumStringContainer&rhs) const;

public:
  std::string ToString() const;
  const char* ToCharArray() const;

private:
  std::string str;
};

EnumStringContainer::EnumStringContainer(const std::string &_str)
{
   this->str = _str;
}

bool EnumStringContainer::operator==(const EnumStringContainer &rhs) const
{
   return this->ToString() == rhs.ToString();
}

std::string EnumStringContainer::ToString() const
{
   return this->str;
}

const char* EnumStringContainer::ToCharArray() const
{
   return this->str.c_str();
}

// Usage
class Color : public EnumStringContainer
{
private:
    Color(const std::string &color);

public:
    static const Color Red;
    static const Color Blue;
    static const Color Green;
};

// Constructor is private because I don't want new "random" values being created.
Color::Color(const std::string &color) : EnumStringContainer(color)
{
}

const Color Color::Red("Red");
const Color Color::Blue("Blue");
const Color Color::Green("Green");

int main()
{
    Color color = Color::Red;
    return 0;
}

Design Review

I am not sure why you would want to do this.

enum Colours {Red, Green, Blue};

Is much better and type safe.

This code does not have any type safety and thus does not give me any confidence in its usage. Also the comparison is a string compare (not very efficient). Also completely different types are comparable and do not generate a compiler error.

class Color :       public EnumStringContainer {/* STUFF */}
class TypesOfCoral: public EnumStringContainer {/* STUFF */}

if (Color::red == TypesOfCoral::redCoral) {
    std::cout << "It worked\n";
}

Also there is a slight chance that different types could get a positive match!!!!!

The only reason I can see to use this code is for streaming an enum string, but there is no mechanism to convert that stream back into the correct type so you can't read it back in. So that is a fail (and there are better ways to do that).

Code Review

protected:

Protected! Not sure that is ever useful. But I see you are trying to say this class never stands alone and always has to be inherited from. Think I would prefer the curiously recurring template pattern to achieve that result. But Tomato/Tomato (you have to hear it with me changing accent).

Sure the standard constructor. But in most situations are you not going to construct this with a string literal. Which means you are creating several string objects and copying around a lot of data.

  EnumStringContainer(const std::string &_str);

I move use the new move semantics to prevent the copying.

Sure:

  bool operator==(const EnumStringContainer&rhs) const;

But with == usually comes !=.

Sure:

  std::string ToString() const;      // But why return by value.

By returning by value you are forcing a copy.

Can really do this wrong.

  const char* ToCharArray() const;

But not sure why you would need it.

The only reason to use the two above functions is so that you can serialize your enum's easily. So why not have conversion operators or stream operators rather than explicit get the underlying structure.

Also this allows you to stream your object out but not in.

Don't use this->

   this->str = _str;

This is considered good in Java but in C++ it is considered bad practice.

The only reason you need to use this-> is to distinguish between member and local variables. Which means you have local variables with the same name as members, shadowing the members.

Shadowing variables is a real no-no. Because the compiler can't tell when you accidentally do it wrong (using the local and not the member). But you can get the compiler to tell you when you shadow a member. Thus by never shadowing (and getting the compiler to error when you do shadow) your code is actually better as it will have less potential errors in it from using the wrong variable.

In constructors use the initializer list:

EnumStringContainer::EnumStringContainer(const std::string &_str)
{
   this->str = _str;
}

By doing it this way. You force the code to initialize the member with the default constructor. Then you reinitialize it with the assignment operator. Best to just initialize it once.

EnumStringContainer::EnumStringContainer(const std::string& str_)
    str(str_)
{}

Put the & next to the type

In C the * usually goes next to the variable. But in C++ the * and & go next to the type. This is because type information is much more important than in C and this is information about the type.

// So rather than this
EnumStringContainer::EnumStringContainer(const std::string &_str)

// I expect to see:
EnumStringContainer::EnumStringContainer(const std::string& _str)

Most counter arguments against this go along the lines.

// Here p and x are different types.
int*  p,x;

And I will go yes. But show me a coding standard that allows you to define two variables on the same line. Nobody allows this in modern standards. Its one line per variable;

int*  p;
int   x;

Your operator==() is inside your API. I see little use in call ToString().

   return this->ToString() == rhs.ToString();

Member naming

Generally the standard is user defined types have an initial capitol letter. While variables and members have an initial lower case letter. Thus allows us to quickly determine what is a type and what is a function or variable.

Prefer not to use _ as a prefix

The rules for using _ as a prefix are quite complicated (ok only a tiny bit complicated). But the problem is most people don't know them. So best to stay away from using _ as a prefix.

In your code you don't do it wrong so there is nothing wrong with your code. But I bet you don't know why.

Now there is no restrictions on using _ as a suffix.

// I would avoid the underscore here.
EnumStringContainer::EnumStringContainer(const std::string &_str)

// prefer the suffix version as there are no restrictions.
EnumStringContainer::EnumStringContainer(std::string const& str_)

I mention this last as it is just one of those quirks and it does not actually break your code. see: What are the rules about using an underscore in a C++ identifier?


Three class extended to one class I built this 4 classes where the Main is the one responsible for setting and getting the variables. I am using PHP7.1. Please let me know on how to make this more amazing or maybe using trait. How does my classes look? Is it professionally made? what do you guys think? thank you very much!

<?php

class Main
{
    // Parameters
    protected $first_name;
    protected $middle_name;
    protected $last_name;

    public function first_name($first_name)
    {
        $this->first_name = $first_name;
        return $this;
    }

    public function middle_name($middle_name)
    {
        $this->middle_name = $middle_name;
        return $this;
    }

    public function last_name($last_name)
    {
        $this->last_name = $last_name;
        return $this;
    }
}

class Cpc extends Main
{
    public function __construct()
    {
        echo 'this cpc construct
';
    }

    public function generateCpc()
    {
        echo 'this cpc generated
';
        echo $this->first_name." ".$this->middle_name." ".$this->last_name;
        echo '
';
    }
}

class Basetable extends Cpc
{
    public function __construct()
    {
        echo 'this basetable construct
';
    }

    public function generateBasetable()
    {
        echo 'this basetable generated
';
        echo $this->first_name." ".$this->middle_name." ".$this->last_name;
        echo '
';
    }
}

class Layer extends Basetable
{
    public function __construct()
    {
        echo 'this layer construct
';
    }

    public function generateLayer()
    {
        echo 'this layer generated
';
        echo $this->first_name." ".$this->middle_name." ".$this->last_name;
        echo '
';
    }
}

class Master extends Layer
{
    public function __construct()
    {
        echo 'this Master construct
';
    }

    public function generate()
    {
        $this->generateCpc();
        $this->generateBasetable();
        $this->generateLayer();

        echo 'this master generated
';
        echo $this->first_name." ".$this->middle_name." ".$this->last_name;
        echo '
';
    }
}

$o = new Master();
$o->first_name('ken dan')->middle_name('sunico')->last_name('tinio')->generate();

?>

Basically, this structure makes very little sense by itself, without knowing the real purpose of all these classes and the similarities worth extending.

But some suggestions could be made anyway.

whatever class that is intended to display some data, by definition cannot have a property like first name. First names are generally belong to persons. A class must represent a solid entity, not a Frankenstein constructed from different parts. So in your place I would avoid creating such entities as cyborgs, that bear both human-like and machine-like features. Whatever belongs to a person, goes to one class, and whatever belongs to a table renderer belongs to another class.
There is a PHP coding standard, and you are supposed to follow its guidelines.

So, for as little information as provided in your question, here is a class structure I propose for you:

There is a class Person that holds the person's personal information,

class Person
{
    // Parameters
    protected $firstName;
    protected $middleName;
    protected $lastName;

    public function __construct($firstName, $middleName, $lastName)
    {
        $this->firstName = $firstName;
        $this->middleName = $middleName;
        $this->lastName = $lastName;
    }

    public function setFirstName($firstName)
    {
        $this->firstName = $firstName;
        return $this;
    }

    public function setMiddleName($middleName)
    {
        $this->middleName = $middleName;
        return $this;
    }

    public function setLastName($lastName)
    {
        $this->lastName = $lastName;
        return $this;
    }

    public function getFirstName()
    {
        return $this->firstName;
    }

    public function getMiddleName()
    {
        return $this->middleName;
    }

    public function getLastName()
    {
        return $this->lastName;
    }
}

and a class Output that renders tables.

class Output
{
    protected $person;

    public function __construct(Person $person)
    {
        echo 'this Master construct
';
        $this->person = $person;
    }

    protected function generateCpc()
    {
        echo 'this cpc generated
';
        echo $this->person->getFirstName()." ".$this->person->getMiddleName()." ".$this->person->getLlast_name;
        echo '
';
    }

    protected function generateBasetable()
    {
        echo 'this basetable generated
';
        echo $this->person->getFirstName()." ".$this->person->getMiddleName()." ".$this->person->getLlast_name;
        echo '
';
    }

    protected function generateLayer()
    {
        echo 'this layer generated
';
        echo $this->person->getFirstName()." ".$this->person->getMiddleName()." ".$this->person->getLlast_name;
        echo '
';
    }

    public function generate()
    {
        $this->generateCpc();
        $this->generateBasetable();
        $this->generateLayer();

        echo 'this master generated
';
        echo $this->person->getFirstName()." ".$this->person->getMiddleName()." ".$this->person->getLlast_name;
        echo '
';
    }
}

so it can be used like this

$output = new Output(new Person('ken dan', 'sunico', 'tinio'));
$output->generate();


Removing Loop from Numpy XOR I have been playing around with numpy and matplotlib.
My little project was to create a scatter plot ranging from -1 to 1 on both X and Y, but where the shading is done with the XOR scheme.
The following is the code that I have implemented (it is in a cell of a jupyter notebook, hence the trailing ; after plt.scatter):
arr_size = 2000

X = np.random.uniform(low=-1.0, high=1.0, size=(arr_size, arr_size))

colour = np.zeros(arr_size)

for i in range(arr_size):
    if X[0, i] > 0 and X[1, i] < 0:
        colour[i] = 1
    elif X[0, i] < 0 and X[1, i] > 0:
        colour[i] = 1

plt.scatter(X[0], X[1], c=colour);

The output that I generated was:

Which is the desired output.
However, I am on a bit of a campaign to make my numpy code run faster (I am on an ML course and we have been taught to remove for loops wherever possible). Can anyone show me how to make this code more efficient?
Cheers

Since (a < 0 and b > 0) or (a > 0 and b < 0) can be summarized as a*b < 0, moreover * and < work on vectors, we get the one-liner:
plt.scatter(X[0], X[1], c=X[0]*X[1] < 0)


Primitive String trimmer in C I just picked up C and am following through The C programming language.

I've previously got experience with a lot of 'higher'-level languages, so when I saw this exercise in the book:

  Write a program to copy its input to its output, replacing each string of one or more blanks by a
  single blank.

I thought I'd create a very basic and primitive trim-program that takes an input and removes all superfluous white-spaces (more than 1 in a row) and prints out a 'trimmed' string.

As mentioned I just recently started and we haven't covered dynamic arrays yet, so I'm just using static ones.

But I was hoping to get some feedback on my solution:

#include <stdio.h>

/*
** A primitive trimmer
** to remove any superflous
** whitespaces in a given
** string (input).
*/

int main () {

    /*
    ** Setting up the variables
    **
    ** Working with static
    ** arrays since I haven't
    ** been introduced to dynamic.
    */
    char input [100], output[100];
    fgets(input, 100, stdin);
    int i = 0, k = 0;

    // Looping over input until hitting
    // an item containing a null-char.
    while (input[i] != '\0') {
        if (input[i] == ' ') { // hitting first space.
            int j = i + 1;     // initiating temporary counter.
            if (input[j] != '\0') {
                               // find index of first non-space
                               // character after initial space.
                while (input[j] != '\0' && input[j] == ' ') {
                    if (input[j] == ' ') {
                        i++; // keep indexing input-index
                    }
                    j++; // keep incrementing temporary indexer.
                }
            }
        }
        output[k] = input[i]; // insert non-space item into output.
        i++;                 // increment input-indexer.
        k++;                 // prepare output-indexer for next non-space.
    }
    printf("%s", output);     // print trimmed input.
}

It's not going to be pretty and you might not like it, but please keep in mind that I am new to the language!

I'd like to get feedback on my overall solution, any thoughts on the baseline algorithm and how well I'm following best (syntactical) practices.

int vs size_t
Accordingly to the c language specification, it's highly recommendable to use size_t as the type for index variables. So in your case...
int i = 0, k = 0;

should be:
size_t i = 0, k = 0;

if condition repeats while condition
I was a bit confused by this piece of code
while (input[j] != '\0' && input[j] == ' ') {
  if (input[j] == ' ') {
    i++; // keep indexing input-index
  }
  j++; // keep incrementing temporary indexer.
}

Specifically the if condition - it will always be true due to the while condition logic. So, as result, variable j just duplicates i with an invariant j = i+1.

Lack of null character
If you test your code on any string input, you will notice that the program prints some strange characters after your result. It happens because you didn't copy the \0 character to the output array.

Taking all this into an account (as well as @pacmaninbw's and @holroy's recommendations)
#include <stdio.h>
#include <ctype.h>

#define MAX 100

int
main ()
{
  char input [MAX], output[MAX];
  fgets(input, MAX, stdin);
  size_t inputIndex = 0, outputIndex = 0;

  while (input[inputIndex] != '\0')
    {
      output[outputIndex++] = input[inputIndex];
      if (input[inputIndex] == ' ')
        while(input[++inputIndex] == ' ')
          ;
      else
        inputIndex++;
    }
    output[outputIndex] = '\0';
    printf("%s", output);
}


Java: Constructor- and Method-overloading exercise It's from a Java-course I'm enrolled into.

The classes "Instructor" and "Book" are already provided.

You got to write the different constructors of "Instructor" as well as the methods "getMostRecentBookTitle" and overloaded "updateBook".

Here's are the final classes expanded with my additional code:

// ------- Class Instructor ----------------------------------------
public class Instructor {
    public long id;
    public String name;
    public String title;
    public String department;
    public Book[] books;

    public Instructor(long id, String name, String title, String department, Book[] books) {
        this(id, name, title, department);
        this.books = books;
    }

    public Instructor(long id, String name, String title, String department) {
        this(id, name, title);
        this.department = department;
    }

    public Instructor(long id, String name, String title) {
        this(id, name);
        this.title = title;
    }

    public Instructor(long id, String name) {
        this.id = id;
        this.name = name;
    }

    public String getMostRecentBookTitle() {
        return books[books.length - 1].title;
    }

    public Book updateBook(int index, String title) {
        Book retBook = books[0];
        Book[] updatedBooks = new Book[books.length + 1];
        int i = 0;
        int j = 0;

        while (i < updatedBooks.length) {
            if (i == index) {
                updatedBooks[i] = new Book(title);
                retBook = books[i];
            } else {
                updatedBooks[i] = books[j];
            }

            i++;

            if (i != index) {
                j++;
            }
        }

        books = updatedBooks;

        return retBook;
    }

    public Book updateBook(int index, Book book) {
        for (int i = 0; i < books.length; i++) {
            if (i == index) {
                Book oldBook = books[i];
                books[i] = book;
                return oldBook;
            }
        }

        return new Book("");
    }

    public static void main(String[] args) {
        Book book1 = new Book("Wind Energy Design");
        Book book2 = new Book("Aerodynamics");
        Book book3 = new Book("Effective Java");

        Instructor instructor =
            new Instructor( 248,
                            "Walter", "Lecturer", "Mechanical Engineering",
                            new Book[]{book1, book2, book3});
        System.out.println(instructor.getMostRecentBookTitle());
        System.out.println("Title of old book: " + instructor.updateBook(1, "Vector mechanics").getTitle());

        Book book4 = new Book("Thermodynamics");
        System.out.println("Title of old book: " + instructor.updateBook(1, book4).getTitle());
    }
}

// ------- Class Book ----------------------------------------
public class Book {
    public String title;

    public Book(String title) {
        this.title = title;
    }

    public void setTitle(String title) {
        this.title = title;
    }

    public String getTitle() {
        return title;
    }
}

It works well and it has passed the automated tests.

But I'm unsatisfied with the method "updateBook(int index, String title)". That's just too much code for to accomplish such a simple task. In my opinion.

In JavaScript they have these method "splice". I guess that won't work in Java because Array-sizes can't be changed after construction.

Nevertheless: Is there a better way to write that method in Java?

Other hints, comments and recommendations concerning my implementation welcomed too.

In java, you can just set the value at the given index using that index. Thus, starting out with the simpler update book (which uses the book object), this becomes:

public Book updateBook(int index, Book book) {
    Book oldBook = books[index];
    books[index] = book;
    return oldBook;
}

Alike in the ... string title ... method, but now, you can even use the method from before:

public Book updateBook(int index, String title) {
    return updateBook(index, new Book(title));
}

Disclaimer: this has not been typed in an IDE or compiled, but just done on the fly, thus it might include bugs. But the concept should be clear.